Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ReplaceResource overwrite of input resource causes unexpected results #58

Open
nabowler opened this issue Jun 29, 2021 · 1 comment
Open

Comments

@nabowler
Copy link

nabowler commented Jun 29, 2021

Our implementation of the SCIM Server does not return the Address Display field. When replacing a user with multiple addresses that have had their Display field set on the Client side, the Display values of the Addresses are not guaranteed to be correct, or empty, after calling ReplaceResource

Addresses before ReplaceResource

[
  {
    "country": "AU",
    "streetAddress": "123 Stuart St",
    "locality": "State College",
    "region": "Tasmania",
    "postalCode": "16801-6139",
    "type": "MAILING",
    "formatted": "123 Stuart St\n\n State College Tasmania 16801-6139",
    "display": "123 Stuart St\n\n State College Tasmania 16801-6139"
  },
  {
    "country": "US",
    "streetAddress": "123 Stuart St",
    "locality": "State College",
    "region": "PA",
    "postalCode": "16801-6139",
    "type": "PERMANENT",
    "formatted": "123 Stuart St\n\n State College PA 16801-6139",
    "display": "123 Stuart St\n\n State College PA 16801-6139"
  },
  {
    "country": "US",
    "locality": "State College",
    "postalCode": "16801-6139",
    "region": "PA",
    "streetAddress": "300 Science Park Rd",
    "formatted": "300 Science Park Rd\n\n State College PA 16801-6139",
    "addressInvalid": false,
    "type": "UNIVERSITY",
    "display": "300 Science Park Rd\n\n State College PA 16801-6139"
  },
  {
    "country": "US",
    "locality": "University Park",
    "postalCode": "16802",
    "region": "PA",
    "streetAddress": "Brumbaugh Hall 0609",
    "formatted": "Brumbaugh Hall 0609\n\n University Park PA 16802",
    "addressInvalid": false,
    "type": "DORM",
    "key": "194510",
    "display": "Brumbaugh Hall 0609\n\n University Park PA 16802"
  }
]

Addresses after ReplaceResource

[
  {
    "key": "19633",
    "type": "PERMANENT",
    "display": "123 Stuart St\n\n State College Tasmania 16801-6139",
    "country": "US",
    "formatted": "123 Stuart St\nState College, PA 16801-6139 US",
    "locality": "State College",
    "postalCode": "16801-6139",
    "region": "PA",
    "streetAddress": "123 Stuart St"
  },
  {
    "key": "194510",
    "type": "DORM",
    "display": "123 Stuart St\n\n State College PA 16801-6139",
    "country": "US",
    "formatted": "Brumbaugh Hall 0609\nUniversity Park, PA 16802 US",
    "locality": "University Park",
    "postalCode": "16802",
    "region": "PA",
    "streetAddress": "Brumbaugh Hall 0609"
  },
  {
    "key": "170039",
    "type": "MAILING",
    "display": "300 Science Park Rd\n\n State College PA 16801-6139",
    "country": "AU",
    "formatted": "123 Stuart St\nState College, Tasmania 16801-6139 AU",
    "locality": "State College",
    "postalCode": "16801-6139",
    "region": "Tasmania",
    "streetAddress": "123 Stuart St"
  },
  {
    "key": "182012",
    "type": "UNIVERSITY",
    "display": "Brumbaugh Hall 0609\n\n University Park PA 16802",
    "country": "US",
    "formatted": "300 Science Park Rd\nState College, PA 16801-6139 US",
    "locality": "State College",
    "postalCode": "16801-6139",
    "region": "PA",
    "streetAddress": "300 Science Park Rd"
  }
]

The display no longer match the formatted because they are preserved during the json Unmarshal, but the response ordering is arbitrary.

This has the possibility to affect other fields besides display.

@nabowler
Copy link
Author

This snippet is a POC of reflectively instantiating a copy of the Resource to the zero value, unmarshalling into that, then setting the value to the unmarshalled value

package main

import (
	"encoding/json"
	"fmt"
	"reflect"
)

type (
	Doer interface {
		Do()
	}

	Impl struct {
		A string
		B int
	}
)

func main() {
	i := Impl{
		A: "hello",
		B: 300,
	}

	i.Do()

	canIReflect(&i)

	i.Do()
}

func canIReflect(d Doer) {

	v := reflect.ValueOf(d)
	fmt.Printf("%v\n", v)
	vi := reflect.Indirect(v)

	v2 := reflect.New(vi.Type())
	fmt.Printf("%v\n", v2)

	err := json.Unmarshal([]byte(`{"A":"foo"}`), v2.Interface())
	if err != nil {
		panic(err)
	}
	fmt.Printf("%v\n", v2)

	v.Elem().Set(reflect.Indirect(v2))
}

func (i Impl) Do() {
	fmt.Printf("%s %d\n", i.A, i.B)
}
$go run main.go
hello 300
&{hello 300}
&{ 0}
&{foo 0}
foo 0

nabowler added a commit to nabowler/scim-client that referenced this issue Jun 30, 2021
…ource to avoid issues with non-returned fields

Hopefully will fix PennState#58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant