cloudfoundry / stratos

Stratos: Web-based Management UI for Cloud Foundry and Kubernetes
Apache License 2.0
251 stars 132 forks source link

UAA OAuth2 defaults are an OAuth anti-pattern #2612

Open aeijdenberg opened 6 years ago

aeijdenberg commented 6 years ago

A new installation of Stratos deployed on CloudFoundry defaults to using a client ID of cf.

Not only does this not work (until the cf client is modified in UAA to allow a redirect to the Stratos installation), it is not the way OAuth is intended to work

The cf client which ships by default is intended for the cf CLI binary and amongst other things has no client secret set (which is normal for an OAuth client that is widely distributed).

We should instead document and encourage users to create a new client ID for their Stratos console.

Since the Stratos console is a client of type web application (which ends up storing a database of refresh tokens), it should be configured with an appropriate client secret. A reason why this is useful, is that in the event of a leak of the database of refresh tokens, is that they can be made invalid by simply rotating the client secret within UAA.

I suggest at minimum the following changes, that I'd be happy to implement if the approach agreed:

  1. Change initialiseConsoleConfig() and/or GetClientId() signature and implementation to return an error if CONSOLE_CLIENT / CF_CLIENT is not set (instead of defaulting to cf).

  2. Change initialiseConsoleConfig() to error if CONSOLE_CLIENT_SECRET is not set, and remove incorrect advice that // Special case, mostly this is blank, so assume its blank.

  3. SKIP_SSL_VALIDATION=true appears to be set by default in deploy/cloud-foundry/config.properties. It's not safe to ship software that is insecure by default. I think it's fine to allow skipping of SSL validation for various test / debug reasons, but that needs to be opt-in, not the default.

  4. Add documentation for how to create an appropriate UAA OAuth client with redirects / client secret etc set appropriately.

Let me know if you'd be open to a PR for the above.

drnic commented 6 years ago

I agree with the premise of requiring a dedicated stratos UAA client with an explicit secret for a client web application.

Question: since stratos is designed to work against internal CF + public CF (such as https://api.run.pivotal.io) - would this requirement mean that we'd need to ask Pivotal to add a stratos UAA client and tell us the secret? Is the use of the universally adopted cf client a workaround to this?

Unfortunately the UAA does not make it possible for non-admin users to create their own clients (which is possible on nearly all other OAuth systems such as https://github.com/settings/developers).

drnic commented 6 years ago

Chatting on s Slack with @aeijdenberg I now realize we’ve moved to authenticating via UAA/auth token grants; so the uaa client needs to include redirect_uri back to our stratos app.

This means we’re pretty much giving up on the “use stratos against public CF” story?

nwmac commented 6 years ago

@aeijdenberg Yes please - go ahead and open a PR.

@drnic We will be supporting the existing login mechanism as well as SSO using the UAA login. If you are using SSO, then you will need to ensure you have a client set up with the Stratos redirect URI included in the whitelist.

aeijdenberg commented 6 years ago

@nwmac - we're still keen to work on this. In prep I'm doing some tidy up that I think will make the Go part of the code base simpler to maintain (and thus contribute to).

See: https://github.com/cloudfoundry-incubator/stratos/pull/2662 - proposed Go code reorg so that standard IDEs and Go tooling can be used with this codebase

and: https://github.com/cloudfoundry-incubator/stratos/issues/2663 - changes for more consistent handling of runtime properties

I hope those are helpful - it would be good to get some initial feedback on the general direction before going too much further.

The main problem we're trying to solve with both of those is speed to develop (much faster feedback loop when IDE can compile your code!) and ease of deploying/testing (we find user provided services a good mechanism for management of secrets, which the console has many of, e.g. client secret for OAuth, session encryption keys (which I suspect most operators are incorrectly leaving to the defaults) etc...)

Once we get a good robust base, it should be easier to contribute improvements to.