fix(group_scim): correct the GroupSCIMIdentity json tag to extern_uid
What does this MR do?
GroupSCIMIdentity declares its first field as ExternalUID string with the tag json:"external_uid", but GitLab sends that key as extern_uid. The tag matches nothing in the response body, so the field decodes as the empty string on every call to GetSCIMIdentitiesForGroup and GetSCIMIdentity. It has never held a value.
Both of those endpoints render through one Grape entity, ee/lib/api/entities/identity_detail.rb, which exposes exactly three keys: extern_uid, user_id and active. The routes in ee/lib/api/provider_identity.rb present both GET /groups/:id/scim/identities and GET /groups/:id/scim/:uid with that entity. The SCIM API documentation lists the same response attribute as extern_uid and shows it under that name in both example responses.
The rest of the library already agrees with GitLab here. UpdateSCIMIdentityOptions, thirty lines below in this same file, spells the request parameter extern_uid, and every other response struct in the SDK that carries this key tags it extern_uid: User, UserIdentity and SCIMIdentity in users.go, and GroupMemberSAMLIdentity in group_members.go. GroupSCIMIdentity was the only one spelling it external_uid.
On the Go field name
I kept the exported field as ExternalUID and changed only the tag, so this fixes decoding and breaks no caller.
The alternative is renaming it to ExternUID as well, which would match GitLab's own spelling, the UpdateSCIMIdentityOptions.ExternUID field in this file, and SCIMIdentity.ExternUID in users.go, which models the same concept. The argument for doing that is unusually strong, because the field being renamed has never held a value: no correct program can depend on what it returns, only on the name compiling.
I still chose the smaller change, because a rename is a source-breaking change to a released major version and the gain is consistency rather than behaviour, while the tag fix is what actually makes the endpoint usable. If you would rather align the name now, say so and I will add the rename to this merge request.
I found this while building gitlab-mcp-server, an MCP server that exposes GitLab through this SDK, by comparing its structs against a record of what each Grape entity exposes taken from a booted gitlab/gitlab-ee:latest at 19.3.1-ee.
Is this a breaking change?
Not at the source level: the exported field keeps its name and its type, so every existing caller still compiles. The behaviour does change, in the only direction it can. ExternalUID starts carrying the value GitLab sends instead of the empty string it has always decoded to, and no caller can have been depending on a field that was never populated.
How was this tested?
The two GET tests passed against inline fixtures whose key was external_uid, a key GitLab never sends. Because the fixture agreed with the struct rather than with the API, the tests were green while the field came back empty on every real call.
Both fixtures now carry extern_uid, which is what the entity above renders. The assertions still compare the whole decoded struct, so they now fail without the tag change, with ExternalUID: "" against the expected UID. I checked that by reverting the tag alone and running them:
--- FAIL: TestGroupSCIM_GetSCIMIdentity (0.00s)
expected: &gitlab.GroupSCIMIdentity{ExternalUID:"be20d8dcc028677c931e04f387", UserID:48, Active:true}
actual : &gitlab.GroupSCIMIdentity{ExternalUID:"", UserID:48, Active:true}
--- FAIL: TestGroupSCIM_GetSCIMIdentitiesForGroup (0.00s)I also added the GIVEN/WHEN/THEN comments the contributor guidelines ask for, and a length assertion on the list so the list test cannot pass on an empty page.
go build ./..., go vet ./..., gofumpt -l, golangci-lint run and the full test suite under -race are all clean. No interface signature changed, so the generated mocks need no regeneration.
Related to #2300