Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Support provider specific configs #118

Merged
merged 9 commits into from
Jun 17, 2020
Merged

Support provider specific configs #118

merged 9 commits into from
Jun 17, 2020

Conversation

tvoran
Copy link
Member

@tvoran tvoran commented Jun 12, 2020 •
edited
Loading

Overview

Adds a provider_config field and initial interface definitions to allow for
custom provider configs to be set.

Also updates go to 1.14.4 in the circleci config.

Design of Change

Provider-specific handling can be added by writing an object that conforms to one or more interfaces in provider_config.go. Some interfaces will be required, like CustomProvider, and others will be invoked if present during the login process (e.g. GroupsFetcher). The interfaces themselves will be small (usually a single method) as it is expected that the parts of the login that need specialization will be different per provider. This pattern allows us to start with a minimal set and add interfaces as necessary.

If a custom provider is configured on the backend object and satisfies a given interface, the interface will be used during the relevant part of the login flow. e.g. after an ID token has been received, the custom provider's UserInfoFetcher interface will be used, if present, to fetch and merge additional identity data.

The custom handlers will be standalone objects defined in their own file (one per provider). They'll be part of the main jwtauth package to avoid potential circular import issues.

Related Issues/Pull Requests

Contributor Checklist

  • Add relevant docs to upstream Vault repository, or sufficient reasoning why docs won’t be added yet: upstream docs will be added when actual providers are added in separate PRs.
  • Add output for any tests not ran in CI to the PR description (eg, acceptance tests)
`make test` output 
$ make test                                                                                 
VAULT_ACC=1 go test -tags='vault-plugin-auth-jwt' $(go list ./... | grep -v /vendor/) -v  -timeout 10m
=== RUN   TestGetClaim
--- PASS: TestGetClaim (0.00s)
=== RUN   TestExtractMetadata
--- PASS: TestExtractMetadata (0.00s)
=== RUN   TestValidateAudience
--- PASS: TestValidateAudience (0.00s)
=== RUN   TestValidateBoundClaims
--- PASS: TestValidateBoundClaims (0.00s)
=== RUN   Test_normalizeList
--- PASS: Test_normalizeList (0.00s)
=== RUN   TestParseHelp
=== RUN   TestParseHelp/#00
=== RUN   TestParseHelp/#01
=== RUN   TestParseHelp/#02
=== RUN   TestParseHelp/#03
=== RUN   TestParseHelp/#04
=== RUN   TestParseHelp/#05
--- PASS: TestParseHelp (0.00s)
    --- PASS: TestParseHelp/#00 (0.00s)
    --- PASS: TestParseHelp/#01 (0.00s)
    --- PASS: TestParseHelp/#02 (0.00s)
    --- PASS: TestParseHelp/#03 (0.00s)
    --- PASS: TestParseHelp/#04 (0.00s)
    --- PASS: TestParseHelp/#05 (0.00s)
=== RUN   TestConfig_JWT_Read
--- PASS: TestConfig_JWT_Read (0.00s)
=== RUN   TestConfig_JWT_Write
--- PASS: TestConfig_JWT_Write (0.00s)
=== RUN   TestConfig_JWKS_Update
--- PASS: TestConfig_JWKS_Update (0.00s)
=== RUN   TestConfig_JWKS_Update_Invalid
--- PASS: TestConfig_JWKS_Update_Invalid (0.00s)
=== RUN   TestConfig_ResponseMode
--- PASS: TestConfig_ResponseMode (0.00s)
=== RUN   TestConfig_OIDC_Write
--- PASS: TestConfig_OIDC_Write (0.35s)
=== RUN   TestConfig_OIDC_Write_ProviderConfig
=== RUN   TestConfig_OIDC_Write_ProviderConfig/valid_provider_config
=== RUN   TestConfig_OIDC_Write_ProviderConfig/unknown_provider_in_provider_config
=== RUN   TestConfig_OIDC_Write_ProviderConfig/provider_config_missing_provider
=== RUN   TestConfig_OIDC_Write_ProviderConfig/provider_config_not_set
--- PASS: TestConfig_OIDC_Write_ProviderConfig (0.12s)
    --- PASS: TestConfig_OIDC_Write_ProviderConfig/valid_provider_config (0.03s)
    --- PASS: TestConfig_OIDC_Write_ProviderConfig/unknown_provider_in_provider_config (0.03s)
    --- PASS: TestConfig_OIDC_Write_ProviderConfig/provider_config_missing_provider (0.03s)
    --- PASS: TestConfig_OIDC_Write_ProviderConfig/provider_config_not_set (0.03s)
=== RUN   TestLogin_JWT
--- PASS: TestLogin_JWT (0.03s)
=== RUN   TestLogin_Leeways
--- PASS: TestLogin_Leeways (0.18s)
=== RUN   TestLogin_OIDC
2020-06-15T16:17:53.484-0700 [WARN]  unable to locate /org/primary in claims: /org/primary at part 0: couldn't find key "org"
--- PASS: TestLogin_OIDC (0.32s)
=== RUN   TestLogin_NestedGroups
--- PASS: TestLogin_NestedGroups (0.00s)
=== RUN   TestLogin_OIDC_StringGroupClaim
2020-06-15T16:17:53.698-0700 [WARN]  unable to locate /org/primary in claims: /org/primary at part 0: couldn't find key "org"
--- PASS: TestLogin_OIDC_StringGroupClaim (0.21s)
=== RUN   TestLogin_JWKS_Concurrent
--- PASS: TestLogin_JWKS_Concurrent (0.26s)
=== RUN   TestOIDC_AuthURL
=== RUN   TestOIDC_AuthURL/normal_case
=== PAUSE TestOIDC_AuthURL/normal_case
=== RUN   TestOIDC_AuthURL/missing_role
=== PAUSE TestOIDC_AuthURL/missing_role
=== RUN   TestOIDC_AuthURL/valid_redirect_uri
=== PAUSE TestOIDC_AuthURL/valid_redirect_uri
=== RUN   TestOIDC_AuthURL/invalid_redirect_uri
=== PAUSE TestOIDC_AuthURL/invalid_redirect_uri
=== CONT  TestOIDC_AuthURL/normal_case
=== CONT  TestOIDC_AuthURL/valid_redirect_uri
=== CONT  TestOIDC_AuthURL/invalid_redirect_uri
=== CONT  TestOIDC_AuthURL/missing_role
2020-06-15T16:17:53.987-0700 [WARN]  unauthorized redirect_uri: redirect_uri=http://bitc0in-4-less.cx
--- PASS: TestOIDC_AuthURL (0.02s)
    --- PASS: TestOIDC_AuthURL/invalid_redirect_uri (0.00s)
    --- PASS: TestOIDC_AuthURL/valid_redirect_uri (0.03s)
    --- PASS: TestOIDC_AuthURL/missing_role (0.03s)
    --- PASS: TestOIDC_AuthURL/normal_case (0.03s)
=== RUN   TestOIDC_Callback
=== RUN   TestOIDC_Callback/successful_login
=== RUN   TestOIDC_Callback/failed_login_-_bad_nonce
=== RUN   TestOIDC_Callback/failed_login_-_bound_claim_mismatch
=== RUN   TestOIDC_Callback/missing_state
=== RUN   TestOIDC_Callback/unknown_state
=== RUN   TestOIDC_Callback/valid_state,_missing_code
=== RUN   TestOIDC_Callback/failed_code_exchange
=== RUN   TestOIDC_Callback/no_response_from_provider
=== RUN   TestOIDC_Callback/test_bad_address
=== RUN   TestOIDC_Callback/test_invalid_client_id
=== RUN   TestOIDC_Callback/client_nonce
--- PASS: TestOIDC_Callback (0.05s)
    --- PASS: TestOIDC_Callback/successful_login (0.01s)
    --- PASS: TestOIDC_Callback/failed_login_-_bad_nonce (0.01s)
    --- PASS: TestOIDC_Callback/failed_login_-_bound_claim_mismatch (0.01s)
    --- PASS: TestOIDC_Callback/missing_state (0.00s)
    --- PASS: TestOIDC_Callback/unknown_state (0.00s)
    --- PASS: TestOIDC_Callback/valid_state,_missing_code (0.00s)
    --- PASS: TestOIDC_Callback/failed_code_exchange (0.00s)
    --- PASS: TestOIDC_Callback/no_response_from_provider (0.00s)
    --- PASS: TestOIDC_Callback/test_bad_address (0.00s)
    --- PASS: TestOIDC_Callback/test_invalid_client_id (0.00s)
    --- PASS: TestOIDC_Callback/client_nonce (0.01s)
=== RUN   TestOIDC_ValidRedirect
--- PASS: TestOIDC_ValidRedirect (0.00s)
=== RUN   TestParseMount
--- PASS: TestParseMount (0.00s)
=== RUN   TestPath_Create
=== RUN   TestPath_Create/happy_path
=== RUN   TestPath_Create/no_user_claim
=== RUN   TestPath_Create/no_binding
=== RUN   TestPath_Create/has_bound_subject
=== RUN   TestPath_Create/has_audience
=== RUN   TestPath_Create/has_cidr
=== RUN   TestPath_Create/has_bound_claims
=== RUN   TestPath_Create/has_expiration,_not_before_custom_leeways
=== RUN   TestPath_Create/storing_zero_leeways
=== RUN   TestPath_Create/storing_negative_leeways
=== RUN   TestPath_Create/storing_an_invalid_bound_claim_type
=== RUN   TestPath_Create/role_with_invalid_glob_in_claim
=== RUN   TestPath_Create/role_with_invalid_glob_in_claim_array
--- PASS: TestPath_Create (0.00s)
    --- PASS: TestPath_Create/happy_path (0.00s)
    --- PASS: TestPath_Create/no_user_claim (0.00s)
    --- PASS: TestPath_Create/no_binding (0.00s)
    --- PASS: TestPath_Create/has_bound_subject (0.00s)
    --- PASS: TestPath_Create/has_audience (0.00s)
    --- PASS: TestPath_Create/has_cidr (0.00s)
    --- PASS: TestPath_Create/has_bound_claims (0.00s)
    --- PASS: TestPath_Create/has_expiration,_not_before_custom_leeways (0.00s)
    --- PASS: TestPath_Create/storing_zero_leeways (0.00s)
    --- PASS: TestPath_Create/storing_negative_leeways (0.00s)
    --- PASS: TestPath_Create/storing_an_invalid_bound_claim_type (0.00s)
    --- PASS: TestPath_Create/role_with_invalid_glob_in_claim (0.00s)
    --- PASS: TestPath_Create/role_with_invalid_glob_in_claim_array (0.00s)
=== RUN   TestPath_OIDCCreate
=== RUN   TestPath_OIDCCreate/both_explicit_and_default_role_type
=== RUN   TestPath_OIDCCreate/invalid_reserved_metadata_key_role
=== RUN   TestPath_OIDCCreate/invalid_duplicate_metadata_destination
=== RUN   TestPath_OIDCCreate/custom_expiration_leeway_and_not_before_leeway_values
--- PASS: TestPath_OIDCCreate (0.00s)
    --- PASS: TestPath_OIDCCreate/both_explicit_and_default_role_type (0.00s)
    --- PASS: TestPath_OIDCCreate/invalid_reserved_metadata_key_role (0.00s)
    --- PASS: TestPath_OIDCCreate/invalid_duplicate_metadata_destination (0.00s)
    --- PASS: TestPath_OIDCCreate/custom_expiration_leeway_and_not_before_leeway_values (0.00s)
=== RUN   TestPath_Read
--- PASS: TestPath_Read (0.00s)
=== RUN   TestPath_Delete
--- PASS: TestPath_Delete (0.00s)
=== RUN   TestNewProviderConfig
=== RUN   TestNewProviderConfig/normal_case
=== RUN   TestNewProviderConfig/no_provider_config
=== RUN   TestNewProviderConfig/provider_field_not_present_in_provider_config
=== RUN   TestNewProviderConfig/unknown_provider
=== RUN   TestNewProviderConfig/provider_name_not_present_in_provider_config
=== RUN   TestNewProviderConfig/error_in_Initialize
--- PASS: TestNewProviderConfig (0.00s)
    --- PASS: TestNewProviderConfig/normal_case (0.00s)
    --- PASS: TestNewProviderConfig/no_provider_config (0.00s)
    --- PASS: TestNewProviderConfig/provider_field_not_present_in_provider_config (0.00s)
    --- PASS: TestNewProviderConfig/unknown_provider (0.00s)
    --- PASS: TestNewProviderConfig/provider_name_not_present_in_provider_config (0.00s)
    --- PASS: TestNewProviderConfig/error_in_Initialize (0.00s)
PASS
ok  	github.com/hashicorp/vault-plugin-auth-jwt	2.140s
?   	github.com/hashicorp/vault-plugin-auth-jwt/cmd/vault-plugin-auth-jwt	[no test files]
  • Backwards compatible

tvoran added 2 commits June 12, 2020 13:14
@tvoran
support provider specific configs
21cea03 
Adds a `provider_config` field and interface definitions to allow for
custom provider configs to be set.

Also updates go to 1.3.10.
@tvoran
simpler provider_config validation
f4c2671 
Check for provider_config len in NewProviderConfig() instead of
outside.
kalafut
kalafut reviewed Jun 15, 2020
View reviewed changes
.circleci/config.yml Outdated Show resolved Hide resolved
kalafut
kalafut reviewed Jun 15, 2020
View reviewed changes
provider_config.go Outdated Show resolved Hide resolved
kalafut
kalafut reviewed Jun 15, 2020
View reviewed changes
provider_config.go Outdated Show resolved Hide resolved
tvoran and others added 4 commits June 15, 2020 14:06
@tvoran
Update provider_config.go
8b49902 
Co-authored-by: Jim Kalafut <[email protected]>
@tvoran
review feedback
2e6ce6a 
using go 1.14.4 and todo for removing empty provider and fixing tests
@tvoran
Merge branch 'provider_config' of github.com:hashicorp/vault-plugin-a…
641bfec 
…uth-jwt into provider_config
@tvoran
fix tests
b67ac3c
@tvoran tvoran marked this pull request as ready for review June 15, 2020 21:29
@tvoran tvoran requested a review from a team June 15, 2020 23:43
catsby
catsby reviewed Jun 16, 2020
View reviewed changes
Copy link
Contributor

@catsby catsby left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great so far! I have some style requests and some questions I'd like to resolve before moving forward.

path_config.go Outdated Show resolved Hide resolved
provider_config.go Outdated Show resolved Hide resolved
provider_config.go Outdated Show resolved Hide resolved
provider_config_test.go Outdated Show resolved Hide resolved
provider_config.go Show resolved Hide resolved
provider_empty.go Show resolved Hide resolved
provider_config.go Outdated Show resolved Hide resolved
path_config.go Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@tvoran tvoran requested a review from catsby June 17, 2020 06:05
catsby
catsby approved these changes Jun 17, 2020
View reviewed changes
Copy link
Contributor

@catsby catsby left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@tvoran tvoran merged commit 71af593 into master Jun 17, 2020
@tvoran tvoran deleted the provider_config branch June 17, 2020 15:37
@tvoran tvoran mentioned this pull request Jun 22, 2020
Merged
5 tasks
@kalafut kalafut mentioned this pull request Jun 29, 2020
Closed
@austingebauer austingebauer mentioned this pull request Jul 10, 2020
Merged
3 tasks
@kalafut kalafut mentioned this pull request Nov 20, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kalafut kalafut kalafut left review comments

@catsby catsby catsby approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tvoran @catsby @kalafut