-
Notifications
You must be signed in to change notification settings - Fork 62
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support for distributed groups claims on Azure #120
Merged
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
fa38e49
Added logic to handle if user is member of more than 200 groups, Azur…
Gaardsholt fb3c46d
Changed return error to not be Azure specific
Gaardsholt f2bc727
Moved azure-specific code to provider_azure.go
tvoran c9376de
Cleanup and refinement
tvoran 3054a10
more tests and cleanup
tvoran 777e0ae
review cleanup
tvoran File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,188 @@ | ||
| package jwtauth | ||
|
|
||
| import ( | ||
| "context" | ||
| "encoding/json" | ||
| "fmt" | ||
| "io/ioutil" | ||
| "net/http" | ||
| "net/url" | ||
| "strings" | ||
|
|
||
| "github.com/coreos/go-oidc" | ||
| log "github.com/hashicorp/go-hclog" | ||
| "golang.org/x/oauth2" | ||
| "golang.org/x/oauth2/clientcredentials" | ||
| ) | ||
|
|
||
| const ( | ||
| // The old MS graph API requires setting an api-version query parameter | ||
| windowsGraphHost = "graph.windows.net" | ||
| windowsAPIVersion = "1.6" | ||
|
|
||
| // Distributed claim fields | ||
| claimNamesField = "_claim_names" | ||
| claimSourcesField = "_claim_sources" | ||
| ) | ||
|
|
||
| // AzureProvider is used for Azure-specific configuration | ||
| type AzureProvider struct { | ||
| // Context for azure calls | ||
| ctx context.Context | ||
|
|
||
| // OIDC provider | ||
| provider *oidc.Provider | ||
| } | ||
|
|
||
| // Initialize anything in the AzureProvider struct - satisfying the CustomProvider interface | ||
| func (a *AzureProvider) Initialize(jc *jwtConfig) error { | ||
| return nil | ||
| } | ||
|
|
||
| // SensitiveKeys - satisfying the CustomProvider interface | ||
| func (a *AzureProvider) SensitiveKeys() []string { | ||
| return []string{} | ||
| } | ||
|
|
||
| // FetchGroups - custom groups fetching for azure - satisfying GroupsFetcher interface | ||
| func (a *AzureProvider) FetchGroups(b *jwtAuthBackend, allClaims map[string]interface{}, role *jwtRole) (interface{}, error) { | ||
| groupsClaimRaw := getClaim(b.Logger(), allClaims, role.GroupsClaim) | ||
|
|
||
| if groupsClaimRaw == nil { | ||
| // If the "groups" claim is missing, it might be because the user is a | ||
| // member of more than 200 groups, which means the token contains | ||
| // distributed claim information. Attempt to look that up here. | ||
| azureClaimSourcesURL, err := a.getClaimSource(b.Logger(), allClaims, role) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("unable to get claim sources: %s", err) | ||
| } | ||
|
|
||
| // Get provider because we'll need to get a new token for microsoft's | ||
| // graph API, specifically the old graph API | ||
| provider, err := b.getProvider(b.cachedConfig) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("unable to get provider: %s", err) | ||
| } | ||
| a.provider = provider | ||
|
|
||
| a.ctx, err = b.createCAContext(b.providerCtx, b.cachedConfig.OIDCDiscoveryCAPEM) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("unable to create CA Context: %s", err) | ||
| } | ||
|
|
||
| azureGroups, err := a.getAzureGroups(azureClaimSourcesURL, b.cachedConfig) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("%q claim not found in token: %v", role.GroupsClaim, err) | ||
| } | ||
| groupsClaimRaw = azureGroups | ||
| } | ||
| b.Logger().Debug(fmt.Sprintf("groups claim raw is %v", groupsClaimRaw)) | ||
| return groupsClaimRaw, nil | ||
| } | ||
|
|
||
| // In Azure, if you are indirectly member of more than 200 groups, they will | ||
| // send _claim_names and _claim_sources instead of the groups, per OIDC Core | ||
| // 1.0, section 5.6.2: | ||
| // https://openid.net/specs/openid-connect-core-1_0.html#AggregatedDistributedClaims | ||
| // In the future this could be used with other providers as well. Example: | ||
| // | ||
| // { | ||
| // "_claim_names": { | ||
| // "groups": "src1" | ||
| // }, | ||
| // "_claim_sources": { | ||
| // "src1": { | ||
| // "endpoint": "https://graph.windows.net...." | ||
| // } | ||
| // } | ||
| // } | ||
| // | ||
| // For this to work, "profile" should be set in "oidc_scopes" in the vault oidc role. | ||
| // | ||
| func (a *AzureProvider) getClaimSource(logger log.Logger, allClaims map[string]interface{}, role *jwtRole) (string, error) { | ||
| // Get the source key for the groups claim | ||
| name := fmt.Sprintf("/%s/%s", claimNamesField, role.GroupsClaim) | ||
| groupsClaimSource := getClaim(logger, allClaims, name) | ||
| if groupsClaimSource == nil { | ||
| return "", fmt.Errorf("unable to locate groups claim %q in %s", role.GroupsClaim, claimNamesField) | ||
| } | ||
| // Get the endpoint source for the groups claim | ||
| endpoint := fmt.Sprintf("/%s/%s/endpoint", claimSourcesField, groupsClaimSource.(string)) | ||
| val := getClaim(logger, allClaims, endpoint) | ||
| if val == nil { | ||
| return "", fmt.Errorf("unable to locate %s in claims", endpoint) | ||
| } | ||
| logger.Debug(fmt.Sprintf("found Azure Graph API endpoint for group membership: %v", val)) | ||
| return fmt.Sprintf("%v", val), nil | ||
| } | ||
|
|
||
| // Fetch user groups from the Azure AD Graph API | ||
| func (a *AzureProvider) getAzureGroups(groupsURL string, c *jwtConfig) (interface{}, error) { | ||
| urlParsed, err := url.Parse(groupsURL) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to parse distributed groups source url %s: %s", groupsURL, err) | ||
| } | ||
| token, err := a.getAzureToken(c, urlParsed.Host) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("unable to get token: %s", err) | ||
| } | ||
| payload := strings.NewReader("{\"securityEnabledOnly\": false}") | ||
| req, err := http.NewRequest("POST", groupsURL, payload) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("error constructing groups endpoint request: %s", err) | ||
| } | ||
| req.Header.Add("content-type", "application/json") | ||
| req.Header.Add("authorization", fmt.Sprintf("Bearer %s", token)) | ||
|
|
||
| // If endpoint is the old windows graph api, add api-version | ||
| if urlParsed.Host == windowsGraphHost { | ||
| query := req.URL.Query() | ||
| query.Add("api-version", windowsAPIVersion) | ||
| req.URL.RawQuery = query.Encode() | ||
| } | ||
| client := http.DefaultClient | ||
| if c, ok := a.ctx.Value(oauth2.HTTPClient).(*http.Client); ok { | ||
| client = c | ||
| } | ||
| res, err := client.Do(req) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("unable to call Azure AD Graph API: %s", err) | ||
| } | ||
| defer res.Body.Close() | ||
| body, err := ioutil.ReadAll(res.Body) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to read Azure AD Graph API response: %s", err) | ||
| } | ||
| if res.StatusCode != http.StatusOK { | ||
| return nil, fmt.Errorf("failed to get groups: %s", string(body)) | ||
| } | ||
|
|
||
| var target azureGroups | ||
| if err := json.Unmarshal(body, &target); err != nil { | ||
| return nil, fmt.Errorf("unabled to decode response: %s", err) | ||
| } | ||
| return target.Value, nil | ||
| } | ||
|
|
||
| // Login to Azure, using client id and secret. | ||
| func (a *AzureProvider) getAzureToken(c *jwtConfig, host string) (string, error) { | ||
| config := &clientcredentials.Config{ | ||
| ClientID: c.OIDCClientID, | ||
| ClientSecret: c.OIDCClientSecret, | ||
| TokenURL: a.provider.Endpoint().TokenURL, | ||
| Scopes: []string{ | ||
| "openid", | ||
| "profile", | ||
| "https://" + host + "/.default", | ||
| }, | ||
| } | ||
| token, err := config.Token(a.ctx) | ||
| if err != nil { | ||
| return "", fmt.Errorf("failed to fetch Azure token: %s", err) | ||
| } | ||
| return token.AccessToken, nil | ||
| } | ||
|
|
||
| type azureGroups struct { | ||
| Value []interface{} `json:"value"` | ||
| } |
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm seeing that the custom provider
Initialize()method withinNewProviderConfig()will end up being called upon each login. I think that's okay given that the azure provider doesn't do much there. I'm wondering if we would still want that to happen if there is a custom provider with more or potentially slow initialization code. It appears that the customer provider is already initialized when the backend gets its config.Not a blocker. Just wanted to see what your thoughts were :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question, I guess I envisioned the Initialize() method would mainly be used to translate the "provider_config" values into the CustomProvider struct, and not actually run any API calls against a provider (azure in this case). But it's something to keep in mind as this evolves for sure.