PennState/scim-client

ReplaceResource overwrite of input resource causes unexpected results

Opened this issue · 1 comments

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.

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