Repository navigation
x/samlsp: add Assertion.SubjectKey, a stable key for the user - #999
Merged
Merged
Conversation
Assertion offered Subject, the NameID's value, and nothing that safely identifies a user across logins. x/samlsp requests a transient NameID by default, which changes with every login, and even a persistent NameID is unique only together with the IdP that issued it and its qualifiers (saml-core §2.2.2, §8.3.7). An application that keys accounts by Subject can merge users of different IdPs or lose accounts. SubjectKeyParts returns Issuer, NameQualifier (Issuer if omitted), SPNameQualifier (the SP's entity ID if omitted), Format (unspecified if omitted) and Value, as a comparable struct; SubjectKey encodes them as "samlsp-subject-v1" followed by ":<length>:<field>" for each, a fixed, documented and unambiguous encoding that can be stored. Issuer is always part of the key, so that one IdP cannot name another's users through NameQualifier. SPProvidedID is left out. A transient NameID, a BaseID (whose xsi:type QName cannot always be resolved once parsed) and a missing value or Issuer give an error that matches ErrNoSubjectKey. To default SPNameQualifier, Assertion gains SPEntityID, the entity ID of the service provider that accepted the assertion, which ParseResponse fills in. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QFDLnyEpWvYnSDCnCib9sD
…pr0lje-samlsp-subject-key # Conflicts: # x/samlsp/assertion.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
samlsp.Assertionexposed onlySubject(the NameID value), which does not safely identify a user across logins.So an app that keys accounts on
Subjectcan merge users from different IdPs or lose accounts.Changes
Assertion.SubjectKeyParts() (SubjectKeyParts, error)returns a comparable{Issuer, NameQualifier, SPNameQualifier, Format, Value}, filling in the defaults:Assertion.SubjectKey() (string, error)encodes those parts assamlsp-subject-v1followed by:<byte length>:<field>for each field. The encoding is fixed, documented, unambiguous whatever the field contents, and versioned.ErrNoSubjectKey:xsi:typecan't always be resolved reliably;Assertion.SPEntityID, filled in byParseResponse.Subjectdoc now points toSubjectKey. doc.go says to key accounts withSubjectKey, and to qualify any attribute used as a key with the Issuer.Compatibility notes
SPEntityID, soSubjectKeyrefuses them unless the NameID carries an SPNameQualifier.Testing
go build ./...,go vet ./...,go test ./...andgo test -race ./x/samlsp/...pass.x/samlsp/subject_key_test.gocovers::and digits;ParseResponse.ExampleAssertion_SubjectKey.Generated by Claude Code