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

adding oauth support to MinIO browser #8075

Closed
wants to merge 2 commits into from

Conversation

kanagarajkm
Copy link
Contributor

@kanagarajkm kanagarajkm commented Aug 14, 2019

Description

Adding OpenID login support to MinIO Browser.

Motivation and Context

Users will be able to login MinIO Browser through the configured Identity Provider.

How to test this PR?

  1. Setup an Identity Provider(wso2, keycloak, Auth0, etc)
  2. Update config.json with openid configuration
  "openid": {
    "jwks": { "url": "<JWKS_URL>" },
    "configURL": "<CONFIC_URL>",
    "policyClaimPrefix: "<POLICY_CLAIM_PREFIX>"
  }

Sample URLs for Keycloak are
configURL - http://localhost:8080/auth/realms/demo/.well-known/openid-configuration
jwks url - http://localhost:8080/auth/realms/demo/protocol/openid-connect/certs

JWT token should include a custom claim for the policy, this is required to create a STS user in MinIO. The name of the custom claim could be either policy or <NAMESPACE_Prefix>policy.
If there is no namespace then policyClaimPrefix can be ingored. For example if the custom claim name is https://min.io/policy then, policyClaimPrefix should be set as https://min.io/

  1. Open MinIO Browser and click Log in with OpenID
  2. Type in Client ID obtained from Identity Provider and ENTER
  3. The user will be redirected the IdP login page
  4. Upon successful login on IdP the user will be automatically logged into MinIO Browser

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • 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)
  • Documentation needed
  • Unit tests needed
  • Functional tests needed (If yes, add mint PR # here: )

pkg/iam/policy/constants.go Outdated Show resolved Hide resolved
pkg/auth/credentials.go Outdated Show resolved Hide resolved
cmd/config-versions.go Outdated Show resolved Hide resolved
@kanagarajkm kanagarajkm force-pushed the sts-browser branch 2 times, most recently from ae615d6 to 9f09fda Compare August 20, 2019 10:58
@kanagarajkm kanagarajkm force-pushed the sts-browser branch 2 times, most recently from d0b7c22 to bc295d2 Compare August 29, 2019 06:03
@harshavardhana
Copy link
Member

can you get this ready for final merge @kanagarajkm with builds everything running? - I am yet to show this to @abperiasamy will do soon.

@kanagarajkm
Copy link
Contributor Author

can you get this ready for final merge @kanagarajkm with builds everything running? - I am yet to show this to @abperiasamy will do soon.

@harshavardhana ok, i can do that.

@kanagarajkm kanagarajkm force-pushed the sts-browser branch 3 times, most recently from 4c4896c to 2149538 Compare September 13, 2019 05:15
@kanagarajkm kanagarajkm changed the title [WIP] adding oauth support to MinIO browser [adding oauth support to MinIO browser Sep 13, 2019
@kanagarajkm kanagarajkm changed the title [adding oauth support to MinIO browser adding oauth support to MinIO browser Sep 13, 2019
@codecov-io
Copy link

codecov-io commented Sep 13, 2019

Codecov Report

Merging #8075 into master will increase coverage by 0.16%.
The diff coverage is 12.5%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #8075      +/-   ##
==========================================
+ Coverage   43.87%   44.04%   +0.16%     
==========================================
  Files         325      303      -22     
  Lines       52891    52093     -798     
==========================================
- Hits        23208    22944     -264     
+ Misses      27525    26991     -534     
  Partials     2158     2158
Impacted Files Coverage Δ
cmd/web-router.go 88% <ø> (ø) ⬆️
cmd/auth-handler.go 62.76% <0%> (+0.37%) ⬆️
cmd/sts-handlers.go 10.66% <0%> (+1.61%) ⬆️
cmd/web-handler-context.go 89.18% <0%> (-2.48%) ⬇️
cmd/utils.go 67.53% <0%> (-3.74%) ⬇️
cmd/iam.go 9.04% <0%> (-0.03%) ⬇️
cmd/web-handlers.go 50.46% <1.88%> (-2.31%) ⬇️
cmd/jwt.go 85.24% <80%> (-1.08%) ⬇️
cmd/disk-usage.go 0% <0%> (-78.95%) ⬇️
cmd/posix-errors.go 40.74% <0%> (-37.45%) ⬇️
... and 88 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8700945...2149538. Read the comment docs.

@kanagarajkm kanagarajkm marked this pull request as ready for review September 13, 2019 06:07
@kanagarajkm
Copy link
Contributor Author

@harshavardhana the problems are resolved and its ready for review.

Copy link
Member

@harshavardhana harshavardhana left a comment

Choose a reason for hiding this comment

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

can you also update the sts documentation on how to configure this?

cmd/web-handlers.go Outdated Show resolved Hide resolved
@kanagarajkm kanagarajkm force-pushed the sts-browser branch 3 times, most recently from 78ba3bc to 51636ae Compare September 25, 2019 10:32
@kanagarajkm
Copy link
Contributor Author

can you also update the sts documentation on how to configure this?

Done, can you please review?

@harshavardhana
Copy link
Member

@kanagarajkm can you provide more instructions on using this with keycloack ?

@kanagarajkm
Copy link
Contributor Author

@harshavardhana added docs for setting up keycloak, please have a look. Updated web-identity.go to add port as an optional input param since keycloak by default runs on port 8080.

@minio-ops
Copy link

Mint Automation

Test Result
mint-compression-xl.sh ✔️
mint-xl.sh ✔️
mint-compression-dist-xl.sh ✔️
mint-compression-fs.sh ✔️
mint-worm.sh ✔️
mint-fs.sh ✔️
mint-gateway-nas.sh ✔️
mint-large-bucket.sh more...
mint-dist-xl.sh more...

8075-27d6961/mint-large-bucket.sh.log:

Running with
SERVER_ENDPOINT:      72.28.97.58:31735
ACCESS_KEY:           minio
SECRET_KEY:           ***REDACTED***
ENABLE_HTTPS:         0
SERVER_REGION:        us-east-1
MINT_DATA_DIR:        /mint/data
MINT_MODE:            full
ENABLE_VIRTUAL_STYLE: 0

To get logs, run 'docker cp 2346bf3b1fc9:/mint/log /tmp/mint-logs'

(1/14) Running aws-sdk-go tests ... done in 2 seconds
(2/14) Running aws-sdk-java tests ... done in 1 seconds
(3/14) Running aws-sdk-php tests ... done in 59 seconds
(4/14) Running aws-sdk-ruby tests ... done in 16 seconds
(5/14) Running awscli tests ... done in 2 minutes and 15 seconds
(6/14) Running healthcheck tests ... done in 0 seconds
(7/14) Running mc tests ... done in 1 minutes and 33 seconds
(8/14) Running minio-dotnet tests ... done in 1 minutes and 26 seconds
(9/14) Running minio-go tests ... FAILED in 3 minutes and 58 seconds
{
  "args": {},
  "duration": 57716,
  "error": "Please reduce your request",
  "function": "testStorageClassMetadataCopyObject()",
  "message": "CopyObject failed on RRS",
  "name": "minio-go: testStorageClassMetadataCopyObject",
  "status": "FAIL"
}

Executed 8 out of 14 tests successfully.

8075-27d6961/mint-dist-xl.sh.log:

Running with
SERVER_ENDPOINT:      72.28.97.56:32366
ACCESS_KEY:           minio
SECRET_KEY:           ***REDACTED***
ENABLE_HTTPS:         0
SERVER_REGION:        us-east-1
MINT_DATA_DIR:        /mint/data
MINT_MODE:            full
ENABLE_VIRTUAL_STYLE: 0

To get logs, run 'docker cp eb28d9bae755:/mint/log /tmp/mint-logs'

(1/14) Running aws-sdk-go tests ... done in 0 seconds
(2/14) Running aws-sdk-java tests ... done in 1 seconds
(3/14) Running aws-sdk-php tests ... done in 44 seconds
(4/14) Running aws-sdk-ruby tests ... done in 2 seconds
(5/14) Running awscli tests ... done in 2 minutes and 11 seconds
(6/14) Running healthcheck tests ... done in 0 seconds
(7/14) Running mc tests ... done in 1 minutes and 15 seconds
(8/14) Running minio-dotnet tests ... done in 53 seconds
(9/14) Running minio-go tests ... done in 1 minutes and 19 seconds
(10/14) Running minio-java tests ... done in 1 minutes and 18 seconds
(11/14) Running minio-js tests ... FAILED in 23 seconds
{
  "name": "minio-js",
  "function": "fPutObject(bucketName, objectName, filePath, metaData)",
  "args": "bucketName:minio-js-test-dd44be49-5af1-4241-ab62-a8c3c6526050, objectName:datafile-65-MB, filePath:/tmp/datafile-65-MB",
  "duration": 4817,
  "status": "FAIL",
  "error": "Error: read ECONNRESET at exports._errnoException (util.js:1020:11) at TCP.onread (net.js:580:26)"
}

Executed 10 out of 14 tests successfully.

@jonasscherer
Copy link

Is it possible to just use existing oidc tokens from some auth headers for the authentication for the MinIO Browser? thx

@kanagarajkm
Copy link
Contributor Author

Is it possible to just use existing oidc tokens from some auth headers for the authentication for the MinIO Browser? thx

@jonasscherer sorry i didn't understand your question, can you please elaborate.

MinIO browser receives the token from the Identity Provider through a redirect and then it uses this token to create an STS user then login as that user.

@jonasscherer
Copy link

@kanagarajkm we are using MinIO behind a auth proxy, which already handles the login, redirect etc.
So when the request reaches the MinIO Browser, the user is already logged in and the corresponding tokens are available through headers. This way Minio just has to verify the existing tokens against the authentication provider and create/login the user.

The question is, can I offer the MinIO browser an authentication header that, if present, logs the user in directly?

@kanagarajkm
Copy link
Contributor Author

@jonasscherer what is the authentication provider used here? does it have support for OpenID Connect?

@jonasscherer
Copy link

@jonasscherer what is the authentication provider used here? does it have support for OpenID Connect?

Yes - it's actually Keycloak :)

@kanagarajkm
Copy link
Contributor Author

@jonasscherer do you see any problems in configuring KeyCloak as openid provider for MinIO and let the MinIO browser take care of rest instead of using an auth proxy?

@jonasscherer
Copy link

@kanagarajkm Unfortunately I see some problems, because we want to offer a single sign-on for several services. In this case the user would have to log in to Minio again...
It's basically just like your workflow, except that the user is already logged in. How do you figure out if a request comes from a logged in user or not?

@kanagarajkm
Copy link
Contributor Author

@jonasscherer let me clarify a few things here.

Let's take for example keycloak is running in https://auth.domain1.com, then you have a MinIO server running on https://minio.domain2.com then another xyz service running on https://xyz.domain3.com

So when a user tries to access https://minio.domain2.com, they are redirected to https://auth.domain1.com where they enter their credentials(this is not MinIO creds). Then they are redirected back to https://minio.domain2.com as logged in user, they don't login explicitly on MinIO, the user doesn't require MinIO creds. This same applies when they access the xyz service running on https://xyz.domain3.com.

@jonasscherer
Copy link

@kanagarajkm This is exactly what I meant :)
My question referred to how MinIO determines that a user is logged in (after redirected from Keycloak). Typically this is achieved by authentication cookies or headers. I would then have to provide the required information. I think it will probably work out of the box. Thanks for your effort, I'm looking forward to try the new feature.

@kanagarajkm
Copy link
Contributor Author

MinIO server verifies the JWT token received from Keycloak using the public certs(JWKS url) of keycloak. Then creates an STS user with the provided policy. This user is now logged in automatically. So its not just about setting the auth header.

what exactly is your auth proxy? Is it keycloak gatekeeper?

@harshavardhana
Copy link
Member

@kanagarajkm can you re-work this PR to work with #8392 such that, we can merge this into this branch as well.

@harshavardhana
Copy link
Member

harshavardhana commented Oct 14, 2019

Send it to branch new-config-admin-v2 on minio

@larsw
Copy link

larsw commented Oct 15, 2019

Hi,

First off, I must say that I very much welcome this PR!

Question 1: Isn't it better to infer the jwks URL from the .well-known/openid-configuration endpoint configured with configURL? E.g. the jwks configuration property is now redundant.

Question 2/advice: Please consider using a battle-hardened oidc package like https://www.npmjs.com/package/oidc-client - it even has a React wrapper: https://www.npmjs.com/package/react-oidc

Question 3: Consider adding a configurable drop-down menu for choosing the OpenID Connect Provider to use, so that the end-user doesn't need to enter the technical client name of it.

@harshavardhana
Copy link
Member

Closing this, this functionality is merged into new-config-admin-v2 branch

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.

None yet

6 participants