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.

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

Filter identity token keys #12780

Merged
merged 9 commits into from
Oct 12, 2021
Merged

Filter identity token keys #12780

merged 9 commits into from
Oct 12, 2021

Conversation

fairclothjm
Copy link
Contributor

@fairclothjm fairclothjm commented Oct 8, 2021

Description

Filter the keys published at the /keys endpoint for identity tokens in use by roles. Right now, if keys are created for the OIDC provider, they’ll be published at /keys for identity tokens even if they’re not used by that system.

Manual Testing

Test script
getkeys() {
  echo
  echo "====================="
  echo "oidc/.well-known/keys"

  curl -s \
    -X GET \
    http://127.0.0.1:8200/v1/identity/oidc/.well-known/keys \
    | jq -r ".keys[].kid"

  echo
  echo "====================="
  echo "oidc/provider/provider1/.well-known/keys"

  curl -s \
    -X GET \
    -H "X-Vault-Token: root" \
    http://127.0.0.1:8200/v1/identity/oidc/provider/provider1/.well-known/keys \
    | jq -r ".keys[]?.kid"

  echo
}

# should be returned by the `oidc/.well-known/keys` endpoint
vault write identity/oidc/key/key1 rotation_period=2m verification_ttl=2m
vault write identity/oidc/role/role1 key=key1 ttl=1m

getkeys


# should be returned by the `oidc/provider/<PROVIDER>/.well-known/keys` endpoint
vault write identity/oidc/key/key2 rotation_period=2m verification_ttl=2m
vault write identity/oidc/client/client1 \
  key=key2 \
  id_token_ttl=1m \
  access_token_ttl=1m
vault write identity/oidc/provider/provider1 \
    allowed_client_ids="*"

getkeys
Output
Success! Data written to: identity/oidc/key/key1
Success! Data written to: identity/oidc/role/role1

=====================
oidc/.well-known/keys
a5cf885d-0414-5e6a-65ea-6ca00727faef
e618fe98-b406-b974-3ad2-fea5e8e5e510

=====================
oidc/provider/provider1/.well-known/keys

Success! Data written to: identity/oidc/key/key2
Success! Data written to: identity/oidc/client/client1
Success! Data written to: identity/oidc/provider/provider1

=====================
oidc/.well-known/keys
a5cf885d-0414-5e6a-65ea-6ca00727faef
e618fe98-b406-b974-3ad2-fea5e8e5e510

=====================
oidc/provider/provider1/.well-known/keys
1d71e84f-c781-3f24-15bb-90a68a8a41fb
65f28c36-e38e-84dc-0a8e-51547ae8ba0f

@vercel vercel bot temporarily deployed to Preview – vault October 8, 2021 20:35 Inactive
@vercel vercel bot temporarily deployed to Preview – vault-storybook October 8, 2021 20:35 Inactive
@vercel vercel bot temporarily deployed to Preview – vault October 11, 2021 14:16 Inactive
@vercel vercel bot temporarily deployed to Preview – vault-storybook October 11, 2021 14:16 Inactive
Copy link
Contributor

@austingebauer austingebauer left a comment

Choose a reason for hiding this comment

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

Looks good! Had an idea for an alternative approach. Let me know your thoughts.

vault/identity_store_oidc.go Outdated Show resolved Hide resolved
vault/identity_store_oidc.go Show resolved Hide resolved
vault/identity_store_oidc.go Outdated Show resolved Hide resolved
Copy link
Contributor

@vinay-gopalan vinay-gopalan left a comment

Choose a reason for hiding this comment

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

I agree with Austin's suggested optimization, but looks good to me otherwise!

@vercel vercel bot temporarily deployed to Preview – vault-storybook October 11, 2021 19:04 Inactive
@vercel vercel bot temporarily deployed to Preview – vault October 11, 2021 19:04 Inactive
@vercel vercel bot temporarily deployed to Preview – vault October 11, 2021 19:07 Inactive
@vercel vercel bot temporarily deployed to Preview – vault-storybook October 11, 2021 19:07 Inactive
Copy link
Contributor

@austingebauer austingebauer left a comment

Choose a reason for hiding this comment

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

One comment on changelog. Otherwise, LGTM 👍

changelog/12780.txt Outdated Show resolved Hide resolved
@vercel vercel bot temporarily deployed to Preview – vault October 12, 2021 11:50 Inactive
@vercel vercel bot temporarily deployed to Preview – vault-storybook October 12, 2021 11:50 Inactive
@fairclothjm fairclothjm merged commit ee40205 into main Oct 12, 2021
@fairclothjm fairclothjm deleted the oidc-filter-keys branch October 12, 2021 16:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants