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

feat: Allow at most one claim based OpenID IDP #16145

Merged
merged 1 commit into from
Nov 29, 2022

Conversation

donatello
Copy link
Member

@donatello donatello commented Nov 29, 2022

Description

This change allows adding upto one JWT Claim based OpenID provider alongside zero or more Role Policy based OpenID providers.

Previously we only supported exactly one of:

  • a single JWT Claim based OpenID provider, or
  • one or more Role Policy based OpenID providers

Motivation and Context

We do not support multiple JWT claim based openid providers as there is currently no defined way to specify which IDP is intended in the STS request. For role policy based openid providers, the RoleARN is always present, and we are always able to tell which IDP was intended.

In this change, we allow at most one claim based openid provider, alongside other role policy based providers. When the STS request comes without a RoleARN, we assume the user intends to use the claim based openid provider to login.

How to test this PR?

Using the dex IDP setup at https://github.com/minio/minio-iam-testing, use the following environment for MinIO:

export MINIO_IDENTITY_OPENID_DISPLAY_NAME="CLAIM BASED IDP"
export MINIO_IDENTITY_OPENID_CLAIM_NAME="groups"
export MINIO_IDENTITY_OPENID_CLIENT_ID="minio-client-app"
export MINIO_IDENTITY_OPENID_CLIENT_SECRET="minio-client-app-secret"
export MINIO_IDENTITY_OPENID_CONFIG_URL="http://localhost:5556/dex/.well-known/openid-configuration"
export MINIO_IDENTITY_OPENID_REDIRECT_URI="http://127.0.0.1:10000/oauth_callback"
export MINIO_IDENTITY_OPENID_SCOPES="groups,openid,profile,email"

export MINIO_IDENTITY_OPENID_CLIENT_ID_2="minio-client-app-2"
export MINIO_IDENTITY_OPENID_CLIENT_SECRET_2="minio-client-app-secret-2"
export MINIO_IDENTITY_OPENID_CONFIG_URL_2="http://localhost:5557/dex/.well-known/openid-configuration"
export MINIO_IDENTITY_OPENID_DISPLAY_NAME_2="ROLE POLICY BASED IDP"
export MINIO_IDENTITY_OPENID_REDIRECT_URI_2="http://127.0.0.1:10000/oauth_callback"
export MINIO_IDENTITY_OPENID_ROLE_POLICY_2="readonly"
export MINIO_IDENTITY_OPENID_SCOPES_2="groups,openid,profile,email"

Login via console and it should work.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Optimization (provides speedup with no functional changes)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • Fixes a regression (If yes, please add commit-id or PR # here)
  • Unit tests added/updated
  • Internal documentation updated
  • Create a documentation update request here

This change allows adding upto one JWT Claim based OpenID provider
alongside zero or more Role Policy based OpenID providers.

Previously we only supported exactly one of:
- a single JWT Claim based OpenID provider, or
- one or more Role Policy based OpenID providers
@minio-trusted
Copy link
Contributor

Mint Automation

Test Result
mint-large-bucket.sh ✔️
mint-fs.sh ✔️
mint-erasure.sh ✔️
mint-dist-erasure.sh ✔️
mint-compress-encrypt-dist-erasure.sh ✔️
mint-pools.sh ✔️
Deleting image on docker hub
Deleting image locally

@harshavardhana harshavardhana merged commit 87cbd41 into minio:master Nov 29, 2022
@donatello donatello deleted the oidc-claim branch November 30, 2022 03:48
djwfyi added a commit to minio/docs that referenced this pull request Dec 1, 2022
…b.com/minio/minio/releases/tag/RELEASE.2022-11-29T23-40-49Z)

- Adds information about the --remove flag dropping data directories as well and running on a per-drive basis [PR
- Adds note that MinIO only supports one JWT based OpenID provider [PR #16145](minio/minio#16145)

Note: There was no Docs issue to track this release.

Other fix:
- Minor correction to Admin Trace
- Clarifies that mc admin idp ldap policy entities command is cumulative, not exclusive when using multiple flags
djwfyi added a commit to minio/docs that referenced this pull request Dec 5, 2022
…b.com/minio/minio/releases/tag/RELEASE.2022-11-29T23-40-49Z)

- Adds information about the --remove flag dropping data directories as well and running on a per-drive basis [PR
- Adds note that MinIO only supports one JWT based OpenID provider [PR #16145](minio/minio#16145)

Note: There was no Docs issue to track this release.

Other fix:
- Minor correction to Admin Trace
- Clarifies that mc admin idp ldap policy entities command is cumulative, not exclusive when using multiple flags
djwfyi added a commit to minio/docs that referenced this pull request Dec 6, 2022
Updates for mc RELEASE.2022-11-17T21-20-39Z:

- Creates `mc admin idp ldap policy entities` section
- Corrects incorrect `--tier` flag in `mc ilm add`
- Adds info to `mc ilm ls` about new tabular format of output

  Closes #645

- Adds info about modifying endpoints for distributed endpoits.

  Partially addresses #644

Updates for Bugfix [minio RELEASE.2022-11-29T23-40-49Z](https://github.com/minio/minio/releases/tag/RELEASE.2022-11-29T23-40-49Z):

- Adds information about the --remove flag dropping data directories as well and running on a per-drive basis [PR
- Adds note that MinIO only supports one JWT based OpenID provider [PR #16145](minio/minio#16145)

Note: There was no Docs issue to track this release.

Other fixes:
- Minor correction to Admin Trace
- Clarifies that mc admin idp ldap policy entities command is cumulative, not exclusive when using multiple flags

- Adds additional `mc admin config` commands.

  Closes #653
djwfyi added a commit to minio/docs that referenced this pull request Dec 6, 2022
Updates docs for various releases through November 2022.

## Updates for mc RELEASE.2022-11-17T21-20-39Z
    
- Creates `mc admin idp ldap policy entities` section
- Corrects incorrect `--tier` flag in `mc ilm add`
- Adds info to `mc ilm ls` about new tabular format of output
    
Closes #645
    
- Adds info about modifying endpoints for distributed endpoits.
    
Partially addresses #644

## Updates for Bugfix [minio
RELEASE.2022-11-29T23-40-49Z](https://github.com/minio/minio/releases/tag/RELEASE.2022-11-29T23-40-49Z)
    
- Adds information about the --remove flag dropping data directories as
well and running on a per-drive basis [PR
16122](minio/minio#16122)
- Adds note that MinIO only supports one JWT based OpenID provider [PR
#16145](minio/minio#16145)
    
Note: There was no Docs issue to track this release.
    
## Other fix:
- Minor correction to Admin Trace
- Clarifies that mc admin idp ldap policy entities command is
cumulative, not exclusive when using multiple flags

## Add additional `mc admin config` commands

- Adds three additional commands. 
- Notes under several commands that environment variables override
config keys.

Closes #653
@bh4t bh4t added the documentation Documentation-only pull request label May 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Documentation-only pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants