fix: add feedback from review
This commit is contained in:
		
							parent
							
								
									9bd6a1306a
								
							
						
					
					
						commit
						a7937a81a7
					
				|  | @ -25,7 +25,7 @@ func CreateTokenToSessionFunc(verify VerifyFunc) TokenToSessionFunc { | ||||||
| 			Verified          *bool    `json:"email_verified"` | 			Verified          *bool    `json:"email_verified"` | ||||||
| 			PreferredUsername string   `json:"preferred_username"` | 			PreferredUsername string   `json:"preferred_username"` | ||||||
| 			Groups            []string `json:"groups"` | 			Groups            []string `json:"groups"` | ||||||
| 			Acr               string   `json:"acr"` | 			ACR               string   `json:"acr"` | ||||||
| 		} | 		} | ||||||
| 
 | 
 | ||||||
| 		idToken, err := verify(ctx, token) | 		idToken, err := verify(ctx, token) | ||||||
|  | @ -50,7 +50,7 @@ func CreateTokenToSessionFunc(verify VerifyFunc) TokenToSessionFunc { | ||||||
| 			User:              claims.Subject, | 			User:              claims.Subject, | ||||||
| 			Groups:            claims.Groups, | 			Groups:            claims.Groups, | ||||||
| 			PreferredUsername: claims.PreferredUsername, | 			PreferredUsername: claims.PreferredUsername, | ||||||
| 			Acr:               claims.Acr, | 			ACR:               claims.ACR, | ||||||
| 			AccessToken:       token, | 			AccessToken:       token, | ||||||
| 			IDToken:           token, | 			IDToken:           token, | ||||||
| 			RefreshToken:      "", | 			RefreshToken:      "", | ||||||
|  |  | ||||||
|  | @ -727,7 +727,6 @@ func (l *LegacyProvider) convert() (Providers, error) { | ||||||
| 		provider.KeycloakConfig = KeycloakOptions{ | 		provider.KeycloakConfig = KeycloakOptions{ | ||||||
| 			Groups: l.KeycloakGroups, | 			Groups: l.KeycloakGroups, | ||||||
| 			Roles:  l.AllowedRoles, | 			Roles:  l.AllowedRoles, | ||||||
| 			ACRs:   l.AcrValues, |  | ||||||
| 		} | 		} | ||||||
| 	case "keycloak": | 	case "keycloak": | ||||||
| 		provider.KeycloakConfig = KeycloakOptions{ | 		provider.KeycloakConfig = KeycloakOptions{ | ||||||
|  |  | ||||||
|  | @ -149,7 +149,6 @@ type KeycloakOptions struct { | ||||||
| 
 | 
 | ||||||
| 	// Role enables to restrict login to users with role (only available when using the keycloak-oidc provider)
 | 	// Role enables to restrict login to users with role (only available when using the keycloak-oidc provider)
 | ||||||
| 	Roles []string `json:"roles,omitempty"` | 	Roles []string `json:"roles,omitempty"` | ||||||
| 	ACRs  string   `json:"acr,omitempty"` |  | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| type AzureOptions struct { | type AzureOptions struct { | ||||||
|  | @ -261,6 +260,8 @@ type OIDCOptions struct { | ||||||
| 	// ExtraAudiences is a list of additional audiences that are allowed
 | 	// ExtraAudiences is a list of additional audiences that are allowed
 | ||||||
| 	// to pass verification in addition to the client id.
 | 	// to pass verification in addition to the client id.
 | ||||||
| 	ExtraAudiences []string `json:"extraAudiences,omitempty"` | 	ExtraAudiences []string `json:"extraAudiences,omitempty"` | ||||||
|  | 	// to pass acr values to the provider
 | ||||||
|  | 	ACRs string `json:"acr,omitempty"` | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| type LoginGovOptions struct { | type LoginGovOptions struct { | ||||||
|  |  | ||||||
|  | @ -28,7 +28,7 @@ type SessionState struct { | ||||||
| 	User              string   `msgpack:"u,omitempty"` | 	User              string   `msgpack:"u,omitempty"` | ||||||
| 	Groups            []string `msgpack:"g,omitempty"` | 	Groups            []string `msgpack:"g,omitempty"` | ||||||
| 	PreferredUsername string   `msgpack:"pu,omitempty"` | 	PreferredUsername string   `msgpack:"pu,omitempty"` | ||||||
| 	Acr               string   `msgpack:"acr,omitempty"` | 	ACR               string   `msgpack:"acr,omitempty"` | ||||||
| 
 | 
 | ||||||
| 	// Internal helpers, not serialized
 | 	// Internal helpers, not serialized
 | ||||||
| 	Clock clock.Clock `msgpack:"-"` | 	Clock clock.Clock `msgpack:"-"` | ||||||
|  | @ -150,7 +150,7 @@ func (s *SessionState) GetClaim(claim string) []string { | ||||||
| 	case "preferred_username": | 	case "preferred_username": | ||||||
| 		return []string{s.PreferredUsername} | 		return []string{s.PreferredUsername} | ||||||
| 	case "acr": | 	case "acr": | ||||||
| 		return []string{s.Acr} | 		return []string{s.ACR} | ||||||
| 	default: | 	default: | ||||||
| 		return []string{} | 		return []string{} | ||||||
| 	} | 	} | ||||||
|  |  | ||||||
|  | @ -61,7 +61,6 @@ func NewKeycloakProvider(p *ProviderData, opts options.KeycloakOptions) *Keycloa | ||||||
| 
 | 
 | ||||||
| 	provider := &KeycloakProvider{ProviderData: p} | 	provider := &KeycloakProvider{ProviderData: p} | ||||||
| 	provider.setAllowedGroups(opts.Groups) | 	provider.setAllowedGroups(opts.Groups) | ||||||
| 	provider.setAllowedACR(opts.ACRs) |  | ||||||
| 	return provider | 	return provider | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
|  |  | ||||||
|  | @ -26,7 +26,6 @@ func NewKeycloakOIDCProvider(p *ProviderData, opts options.Provider) *KeycloakOI | ||||||
| 	} | 	} | ||||||
| 
 | 
 | ||||||
| 	provider.addAllowedRoles(opts.KeycloakConfig.Roles) | 	provider.addAllowedRoles(opts.KeycloakConfig.Roles) | ||||||
| 	provider.setAllowedACR(opts.KeycloakConfig.ACRs) |  | ||||||
| 	return provider | 	return provider | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
|  |  | ||||||
|  | @ -46,6 +46,7 @@ func NewOIDCProvider(p *ProviderData, opts options.OIDCOptions) *OIDCProvider { | ||||||
| 	} | 	} | ||||||
| 
 | 
 | ||||||
| 	p.setProviderDefaults(oidcProviderDefaults) | 	p.setProviderDefaults(oidcProviderDefaults) | ||||||
|  | 	p.setAllowedACR(opts.ACRs) | ||||||
| 	p.getAuthorizationHeaderFunc = makeOIDCHeader | 	p.getAuthorizationHeaderFunc = makeOIDCHeader | ||||||
| 
 | 
 | ||||||
| 	return &OIDCProvider{ | 	return &OIDCProvider{ | ||||||
|  |  | ||||||
|  | @ -268,7 +268,7 @@ func (p *ProviderData) buildSessionFromClaims(rawIDToken, accessToken string) (* | ||||||
| 		{p.UserClaim, &ss.User}, | 		{p.UserClaim, &ss.User}, | ||||||
| 		{p.EmailClaim, &ss.Email}, | 		{p.EmailClaim, &ss.Email}, | ||||||
| 		{p.GroupsClaim, &ss.Groups}, | 		{p.GroupsClaim, &ss.Groups}, | ||||||
| 		{oidcAcrClaim, &ss.Acr}, | 		{oidcAcrClaim, &ss.ACR}, | ||||||
| 		// TODO (@NickMeves) Deprecate for dynamic claim to session mapping
 | 		// TODO (@NickMeves) Deprecate for dynamic claim to session mapping
 | ||||||
| 		{"preferred_username", &ss.PreferredUsername}, | 		{"preferred_username", &ss.PreferredUsername}, | ||||||
| 	} { | 	} { | ||||||
|  |  | ||||||
|  | @ -117,8 +117,7 @@ func (p *ProviderData) EnrichSession(_ context.Context, _ *sessions.SessionState | ||||||
| // This is not used for fine-grained per route authorization rules.
 | // This is not used for fine-grained per route authorization rules.
 | ||||||
| func (p *ProviderData) Authorize(_ context.Context, s *sessions.SessionState) (bool, error) { | func (p *ProviderData) Authorize(_ context.Context, s *sessions.SessionState) (bool, error) { | ||||||
| 	if len(p.AllowedACRs) > 0 { | 	if len(p.AllowedACRs) > 0 { | ||||||
| 		var _, ok = p.AllowedACRs[s.Acr] | 		if _, ok := p.AllowedACRs[s.ACR]; !ok { | ||||||
| 		if !ok { |  | ||||||
| 			return false, nil | 			return false, nil | ||||||
| 		} | 		} | ||||||
| 	} | 	} | ||||||
|  |  | ||||||
|  | @ -128,7 +128,7 @@ func TestProviderDataAuthorize(t *testing.T) { | ||||||
| 
 | 
 | ||||||
| 			session := &sessions.SessionState{ | 			session := &sessions.SessionState{ | ||||||
| 				Groups: tc.groups, | 				Groups: tc.groups, | ||||||
| 				Acr:    tc.userAcr, | 				ACR:    tc.userAcr, | ||||||
| 			} | 			} | ||||||
| 			p := &ProviderData{} | 			p := &ProviderData{} | ||||||
| 			p.setAllowedGroups(tc.allowedGroups) | 			p.setAllowedGroups(tc.allowedGroups) | ||||||
|  |  | ||||||
		Loading…
	
		Reference in New Issue