Syndesis: SSO Enhancements

Created on 11 Sep 2018 · 39Comments · Source: syndesisio/syndesis

This is a...


[X] Feature request
[ ] Regression (a behavior that used to work and stopped working in a new release)
[ ] Bug report  
[ ] Documentation issue or request

Allow the 'authorize access' prompt at first login to be skipped

Description

Syndesis should be configured with a full 'OAuthClient' in OpenShift. This will avoid the 'authorize access' prompt if grantMethod is set to 'auto'.

Epic closemigrated notitriage targe7.x

Source

kcbabo

All 39 comments

@david-martin FYI

kcbabo on 11 Sep 2018

👍1

@zregvart Can you assess the scope and impact this might have? Is it something we could do for 7.2?

heiko-braun on 17 Sep 2018

Would be a research task, if I understand correctly we would need to add a OAuthClient object to our templates and have it configured so it matches the configuration of oauth_proxy (i.e. have the same OAuth client id and secret).

Seems easy enough, I don't understand the full repercussions of this. The documentation is not very helpful.

Ref OAuthClient, REST API

zregvart on 19 Sep 2018

Creating an OauthClient resource like that requires cluster admin privs. Will our operator have the right juice to do it?

chirino on 8 Oct 2018

Is this in scope for 7.2?

kcbabo on 8 Oct 2018

I'm not certain this can be done for 7.2, not at least by definition of not doing new features...

zregvart on 8 Oct 2018

It's a bug, right? :-)

kcbabo on 8 Oct 2018

giphy

zregvart on 9 Oct 2018

So, I've created an OAuthClient like this:

kind: "OAuthClient"
accessTokenMaxAgeSeconds: null 
apiVersion: "oauth.openshift.io/v1"
metadata:
  name: "syndesis-ui" 
respondWithChallenges: false 
secret: "..."
redirectURIs:
  - "https://syndesis.192.168.42.134.nip.io/oauth/callback"

Where secret is the value we pass as --client-secret to oauth_proxy. I needed to perform this as the admin user on minishift.

It didn't work. I'm still prompted to Authorize Access on the /oauth/authorize/approve page.

screenshot from 2018-10-10 12-48-38

I tried all cobinations of respondWithChallenges true and false and setting secret to the value of --client-id and redirectURIs of "https://syndesis.192.168.42.134.nip.io/oauth/callback" and "https://syndesis.192.168.42.134.nip.io/".

Can't get it to work.

zregvart on 10 Oct 2018

@zregvart can you try set a grantMethod of auto

kind: "OAuthClient"
....
grantMethod: auto

https://docs.openshift.com/container-platform/3.10/rest_api/oapi/v1.OAuthClient.html#object-schema
The default for that field seems to be prompt

david-martin on 10 Oct 2018

@david-martin thanks for the tip, unfortunately doesn't seem to help. This is the definition I created:

$ oc get oauthclients syndesis-ui -o yaml
apiVersion: oauth.openshift.io/v1
grantMethod: auto
kind: OAuthClient
metadata:
  creationTimestamp: 2018-10-10T15:57:27Z
  name: syndesis-ui
  resourceVersion: "947463"
  selfLink: /apis/oauth.openshift.io/v1/oauthclients/syndesis-ui
  uid: 2f4e3b4f-cca5-11e8-a7a1-52540027e1dc
redirectURIs:
- https://syndesis.192.168.42.134.nip.io/oauth/callback
secret: eyJhbGciOiJSUzI1NiIs...pOPYLB083nw

The respondWithChallenges: false seems to be missing for some reason. I'll attach the full YAML I created the object with.

This is on:

$ minishift status
Minishift:  Running
Profile:    minishift
OpenShift:  Running (openshift v3.9.0+71543b2-33)
DiskUsage:  72% of 19G (Mounted On: /mnt/sda1)
CacheUsage: 1.951 GB (used by oc binary, ISO or cached images)

syndesis-oauth-client.yaml.txt

zregvart on 10 Oct 2018

@zregvart From your screenshot, it looks like the client id param in the url is set to a serviceaccount oauth client in the syndesis project system:serviceaccount:syndesis:syndesis-oauthclient

Syndesis should be kicking off the oauth flow using the 'full' oauthclient id of syndesis-ui.
Could you try that and see if the prompt appears?

david-martin on 11 Oct 2018

👍1

@david-martin thanks that worked!

So here's what we need to do:

change --client-id parameter of syndesis-oauthproxy deployment config from system:serviceaccount:${OPENSHIFT_PROJECT}:syndesis-oauth-client to syndesis-ui here:
https://github.com/syndesisio/syndesis/blob/b1b5e6ff7220d5c3126bd6bb18acbe2bb15db0c4/install/generator/04-syndesis-oauth-proxy.yml.mustache#L78
create OAuthClient resource:

apiVersion: oauth.openshift.io/v1
grantMethod: auto
kind: OAuthClient
metadata:
  name: syndesis-ui
redirectURIs:
- https://${ROUTE_HOSTNAME}/oauth/callback
secret: ${OPENSHIFT_OAUTH_CLIENT_SECRET}

This cannot be done by a restricted user, that means that the syndesis-operator role would need to be expanded to allow creation of oauthclient resource.

@rhuss @chirino can you comment on this? Do you think this can work with the operator? Any drawbacks you see?

zregvart on 12 Oct 2018

So one thing I just realised is that when there are multiple projects with Syndesis installed the operator would need to patch that OAuthClient to add to redirectURIs $ROUTE_HOSTNAME of each instance.

zregvart on 12 Oct 2018

Could we create multiple oauth clients? Perhaps including the namespace in the name?

chirino on 12 Oct 2018

Could we create multiple oauth clients? Perhaps including the namespace in the name?

Yup, you're thinking that $OPENSHIFT_OAUTH_CLIENT_SECRET will differ for each namespace also?

zregvart on 12 Oct 2018

@rhuss @chirino are we doing this? Does the operator have the privilege to create OAuthClient resource? How will this affect the installation on the trial cluster or the quick start via OpenShift template?

Would it make sense to make this optional?

zregvart on 18 Oct 2018

optional

I would vote for this

david-martin on 18 Oct 2018

@zregvart we can easily add that privilege to the roles we assign to the operator. However I wonder what impact that on a security assessment of the operator. (will the cluster admin give away the perm easily ?)

For the operator installation, we have to add this permission or not. It's hard to make that part optional.

rhuss on 18 Oct 2018

Well if the cluster admin is installing the operator, then I think it's
safe to assume the admin trusts the Fuse Online bits the operator installs
to not be malicious.

On Thu, Oct 18, 2018 at 12:31 PM Roland Huß notifications@github.com
wrote:

@zregvart https://github.com/zregvart we can easily add that privilege
to the roles we assign to the operator. However I wonder what impact that
on a security assessment of the operator. (will the cluster admin give away
the perm easily ?)

For the operator installation, we have to add this permission or not. It's
hard to make that part optional.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/syndesisio/syndesis/issues/3576#issuecomment-431076467,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAGTV8mii1_VgHwayfBpcTka02ECaNZoks5umKz0gaJpZM4WjFKY
.

--
Hiram Chirino
Engineering | Red Hat, Inc.
[email protected] | redhat.com
skype: hiramchirino | twitter: @hiramchirino

chirino on 18 Oct 2018

yeah, sure, but what are the criteria of a cluster-admin for him to install the operator and where is the 'red line' wrt permissions for him ?

Said that I think it safe to just add the required permission to the operator role and let's see whether there will be a problem or not. Otherwise, we will never find out.

rhuss on 18 Oct 2018

👍1

Tentatively, lets try to get this done next week.

zregvart on 19 Oct 2018

Well if the cluster admin is installing the operator, then I think it's safe to assume the admin trusts the Fuse Online bits the operator installs to not be malicious.

The cluster admin would have to install the operator for every user, which means that the provisioning script on OSO needs to have cluster-admin permissions.

Also, I'm not sure how the OAuthClient can be installed with a pure template installation approach as we still have for OSO and presumably still will have until we switch to OLM. Any ideas, @zregvart ?

rhuss on 22 Nov 2018

Said that I think it safe to just add the required permission to the operator role and let's see whether there will be a problem or not. Otherwise, we will never find out.

quoting myself, I think we found quite some issues already. Would it be worth to reconsider the usage of an OAuthClient resource ?

rhuss on 22 Nov 2018

Also, I'm not sure how the OAuthClient can be installed with a pure template installation approach as we still have for OSO and presumably still will have until we switch to OLM. Any ideas, @zregvart ?

the syndesis.yml template does not require OAuthClient, only the syndesis-privileged.yml (used by syndesis-operator)

zregvart on 23 Nov 2018

I think we have some idea on how this works and what are the requirements imposed by it. We can either drop the feature or implement the requirements needed, most notably the requirement to add ClusterRole/ClusterRoleBinding needed for the syndesis-operator to create the OAuthClient resource.

To make it a bit straightforward we could make the syndesis-operator smarter, by testing if the it has the privilege to create OAuthClient resource and based on that choose syndesis.yml or syndesis-privileged.yml template. And make the creation of ClusterRole/ClusterRoleBinding not a part of the syndesis-operator.yml template.

zregvart on 23 Nov 2018

the syndesis.yml template does not require OAuthClient, only the syndesis-privileged.yml (used by syndesis-operator)

The template used by OSO is the one the operator is using. This works that for a release of fuse-online-install we are pulling the operator image and extract the template from there. So 'syndesis.yml' is not really used for template-based installation. Also, if it would, wouldn't then the installation not be the same ?

rhuss on 23 Nov 2018

@zregvart yeah, I agree that we should be smarter in the operator itself. But I'm afraid that for installing the operator alongside with an OAuthClient, there is no support in the OLM (which takes over the installation of operators). OLM knows how to create a deployment and how to setup the roles for the operator, but not much more. So I'm curious how the OLM would solve such an issue.

rhuss on 23 Nov 2018

Interesting titbits from Strimzi:

https://github.com/operator-framework/operator-lifecycle-manager/blob/38adfbb71b9cf312656da68120ec2d8035e7936e/deploy/chart/catalog_resources/rh-operators/amq-streams.v1.0.0.clusterserviceversion.yaml#L20-L30

zregvart on 23 Nov 2018

@rhuss Do you have this on your radar? It is planned for 7.3

heiko-braun on 28 Jan 2019

Nope I hadn't.

rhuss on 29 Jan 2019

@zregvart I lost a bit the overview of this issue. What it is the suggested solution now ? To make the operator smart, right ? to install only when the permissions are there ?

rhuss on 4 Feb 2019

@zregvart Can you take on this issue for 7.3?

heiko-braun on 4 Feb 2019

The outline of the proposed operator functionality in #4156. Not sure if this is doable on this short schedule.

zregvart on 4 Feb 2019

We decided to roll-back this change and document the steps needed to remove the interstitial OAuth authorization screens.

zregvart on 5 Feb 2019

@zregvart @heiko-braun what will happen with this issue? Is it being moved to 7.4 or it is not being done at all?