Resolve distributed claims in idToken #172
Conversation
seanqsun
changed the title
There was a problem hiding this comment.
I think we should change the runGetToken signature to return an error. That way instead of checking if this idToken is nil we can check the returned error.
func (v verificationTest) runGetToken(t *testing.T) (*IDToken, error) {
// ...
}
func (v verificationTest) run(t *testing.T) {
err := v.runGetToken(t)
if err != nil && !v.wantErr {
t.Errorf("%v", err)
}
if err == nil && v.wantErr {
t.Errorf("expected error")
}
}
verify_test.go
Outdated
| "_claim_sources": { | ||
| "src1": {"endpoint": "123", "access_token":"1234"} | ||
| } | ||
| }`, |
There was a problem hiding this comment.
Formatting here looks wonky. Maybe try to clean this up a little?
verify_test.go
Outdated
| }{ | ||
| { | ||
| test: verificationTest{ | ||
| name: "No distributed claims", |
There was a problem hiding this comment.
Subtest names are usually shorter and don't contain spaces. Maybe "NoDistClaims"?
verify_test.go
Outdated
| }, | ||
| { | ||
| test: verificationTest{ | ||
| name: "Distributed claims match. 1 name : 1 source", |
verify_test.go
Outdated
| }, | ||
| "_claim_sources": { | ||
| "src1": {"endpoint": "https://foo", "access_token":"1234"}, | ||
| "src1": {"endpoint": "https://bar", "access_token":"1234"} |
There was a problem hiding this comment.
This seems like an ambiguous test since the endpoint could be "https://foo" or "https://bar" probably better to drop this test case.
| }, | ||
| signKey: newRSAKey(t), | ||
| }, | ||
| want: map[string]claimSource{ |
There was a problem hiding this comment.
This should result in an error, right?
verify_test.go
Outdated
| func TestDistributedClaims(t *testing.T) { | ||
| tests := []struct { | ||
| test verificationTest | ||
| want map[string]claimSource |
There was a problem hiding this comment.
Add a wantErr bool here to indicate that we want the ID token to fail parsing.
verify.go
Outdated
|
|
||
| //step through the token to mp claim names to claim sources" | ||
| for cn, src := range token.ClaimNames { | ||
| if src != "" { |
There was a problem hiding this comment.
probably
if src == "" {
// return error
}
s, ok := token.ClaimsSources[src]
if !ok {
// return error because source name doesn't exist
}
distributedClaims[cn] = s
|
Also having |
seanqsun
force-pushed
the
distributed-claims
branch
from
b780fa6 to
e7de812
Compare
Updates #171
Needs more test cases and functionality. Right now, it only parses out the claims from the ID Token and puts them in
distributedClaims