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

Dashboard Client ID should be globally unique #462

Closed
sttomm opened this issue Mar 1, 2018 · 18 comments
Closed

Dashboard Client ID should be globally unique #462

sttomm opened this issue Mar 1, 2018 · 18 comments
Assignees

Comments

@sttomm
Copy link

sttomm commented Mar 1, 2018

The Dashboard Client section of the spec only says the id must be a non-empty string. I think it must be a globally unique id. The platform will create an OIDC client with that name for the service, right? For not getting conflicts between the clients of the different services, this OIDC client name must be globally unique.

@n3wscott
Copy link
Contributor

n3wscott commented Mar 2, 2018

There is no reason I can think of that the Oauth client id needs to be globally unique.

Here is what the spec says for that id: The id of the Oauth client that the dashboard will use.

It is just the id of the Oauth client. That id could be reused for multiple services from the same broker, or even for all services from brokers provided by a vendor depending on implementation.

If you still have a question or if this did not answer your concern, please reopen this issue.

@n3wscott n3wscott closed this as completed Mar 2, 2018
@sttomm
Copy link
Author

sttomm commented Mar 7, 2018

Thanks for you answer! I am not quite sure about the implementation of this in the platform. So the most important question from my side is, does the platform automatically create the OAuth client? Or must the client be created independently of the catalog update done by the platform? Is this a manual process, or how it is handled in Cloud Foundry or intended in general?

If the platform creates the client, how should conflicts be handled in case of different services using different passwords for the client? The first service wins and the client will be created with this password?

@n3wscott
Copy link
Contributor

n3wscott commented Mar 9, 2018

The OSB spec says these details should be sorted out between the platform and the broker out-of-band.

The OSB spec also does not give guidance on OAuth token lifecycles. The way current implementations are working is unique to each platform. It could be a manual process (providing your platform with a valid access token) or it could be done within the platform via refresh tokens.

@jboyd01 was working on adding some OAuth language to the OSB spec, maybe he'd have some opinions.

I am also not clear on your usage of "password" and mixing that with the question about OAuth client.

@n3wscott
Copy link
Contributor

n3wscott commented Mar 9, 2018

Seems like there is still some questions so I will reopen until this is sorted out.

@n3wscott n3wscott reopened this Mar 9, 2018
@sttomm
Copy link
Author

sttomm commented Mar 12, 2018

Thanks for reopening it, i didn't have the rights to do so.

Sorry for the confusion about the "password". I meant the OAuth client secret. If you allow different services to use the same OAuth Client ID, all of them have to use the same secret. This is fine when the OAuth client is created via a separate process. But as far as i know i.e. Cloud Foundry creates the OAuth client automatically when getting the service catalog from the Service Broker. As the client secret is part of the Dashboard Client definition in the OSB spec, i think this is the intended way on how to use it. Otherwise i can't think of a reason, why the service broker has to provide the OAuth client secret to the platform. Or is there something i am missing?

So i currently think, that this leads to a conflict if different services (perhaps even services from different service brokers or vendors) use the same client id, but different secrets.

@n3wscott
Copy link
Contributor

You know what, I thought you were asking about the platform to broker OAuth but now I see you are asking about the Dashboard Client SSO info. So sorry to have closed it. And you know I have the same question as you now. I will bring this topic up in the next meeting.

@n3wscott
Copy link
Contributor

Requesting that @zrob give us some insight on how this is really used in CF. It is our opinion from the meeting that the Dashboard Client object really should be moved to the response of a provision.

@zrob
Copy link
Member

zrob commented Mar 15, 2018

Currently the dashboard client is intended to be unique to a broker.

In CF the specified client is entered into the OAuth system called UAA that comes with CF. This client allows the broker to accept CF user credentials in a dashboard UI by following an OAuth flow with the UAA. It also enables other workflows such as letting the broker obtain a user token from the UAA that allows the broker to perform CF actions on applications on behalf of the user.

I can imagine this moving to a provision response in 3.0, but it would be good to have some use-cases that lead to that.

To answer some of the questions from comments above:

does the platform automatically create the OAuth client?

CF does create the client when reading in the broker catalog.

Or must the client be created independently of the catalog update done by the platform?

There's nothing that prevents this workflow. The broker would just omit the client from the catalog and orchestrate things amongst brokers and the auth system externally. No opinion on if this is a recommended approach or not.

If you allow different services to use the same OAuth Client ID

It would be interesting to get a use-case for how a "service" uses this ID. When we originally conceived the feature we didn't think of "services" using the ID, but rather of "brokers" using the ID.

@sttomm
Copy link
Author

sttomm commented Mar 16, 2018

I am with you as you say the service broker uses the dashboard client ID. So you would say, that all services of a broker usually use the same dashboard client? From my perspective this would make sense. But in the spec, the dashboard client is defined on service level. So every service of a broker can define a different dashboard client. This can also be implemented by the brokers and the platform, but does it perhaps add a level of complexity that should be avoided?

If you allow different services to use the same OAuth Client ID
What i meant with this is, what happens if services of different brokers use the same client ID? Perhaps the spec is only missing the information, that the dashboard client must be unique for each broker.

@n3wscott
Copy link
Contributor

Currently the dashboard client is intended to be unique to a broker.

And does this mean that the broker is intended to not be shared? So there is a one-to-one mapping of broker instance to platform?

When we originally conceived the feature we didn't think of "services" using the ID, but rather of "brokers" using the ID.

Is dashboard client in CF being used by the platform to do some work that is suppose to be hidden by the broker?

I am with you as you say the service broker uses the dashboard client ID.

I am not. It sounds like the consumer of this client id is suppose to the the broker directly and it is a unintended consequence that it is being returned to the platform. Is that wrong?

What does the platform do with this client ID and how does the platform prevent confusion with a shared broker?

@zrob
Copy link
Member

zrob commented Mar 16, 2018

Thanks @n3wscott and @sttomm. I definitely misspoke in my earlier comment.

According to the spec and the CC implementation the dashboard client is per service, not per broker. The code is a bit hard to follow, but I don't believe there is anything that would prevent a broker from adding a client for one service and then using it for all services if it chose to.

It sounds like the consumer of this client id is suppose to the the broker directly and it is a unintended consequence that it is being returned to the platform. Is that wrong?

The id of the dashboard_client, as hinted at by the name, was originally meant to be used by the broker to facilitate SSO. This would let a cf user log into a broker provided UI using their platform credentials, rather than creating some other account. It is returned to the platform as an optimization for getting the account into the system, see more in the next answer.

What does the platform do with this client ID

CF does not use the client for anything. The reason it is passed into the system, is that CF will create and manage the client in our OAuth store on behalf of the broker. This is an automation that allows brokers to get a user client without any manual steps.

I can't find anything that requires a broker to return a dashboard client. A broker and platform could work out of band to set up OAuth or some other credentials in order to perform various workflows.

and how does the platform prevent confusion with a shared broker?

I'm not sure I understand this one, but maybe this is answered by the unique constraint. A dashboard client can be registered only once in the system by any broker for any one service. There is a fair amount of validation here. We don't want a nefarious broker to guess another broker's client and be able to register with it.

@sttomm
Copy link
Author

sttomm commented Mar 19, 2018

A dashboard client can be registered only once in the system by any broker for any one service. There is a fair amount of validation here. We don't want a nefarious broker to guess another broker's client and be able to register with it.

So the dashboard client must be unique per service in the end, right? Two services of the same broker cannot share their dashboard client either, right? If this is correct, i think extending the spec regarding global uniqueness of the dashboard client id would be the logical consequence. It should not only be an implementation specific solution. As the dashboard clients shall not be reusable by other services and especially not by other service brokers, the uniqueness of the dashboard client would make this clear in the spec.

@n3wscott
Copy link
Contributor

n3wscott commented Mar 20, 2018

I am going to bring this back up in today's meeting again. And if I am not able to get a good answer I am going to host this issue as a topic at the F2F in April.

@sttomm Still looking into this.

@duglin
Copy link
Contributor

duglin commented Mar 20, 2018

Consider moving this to be a CF-specific extension (perhaps in the Profile) - will revisit on next week's call.

@n3wscott
Copy link
Contributor

From the meeting today, we sussed out that CF is using Dashboard Id in concert with GetCatalog as a login call to the CF platform for the Broker so that the Broker may act on behalf of the user.

Nothing was decided, but we think it might make sense for this to move into the profile of CF and continue to exist for GetCatalog calls for CF platforms as a additional field in the GetCatalog response per the language that is being introduced by #436. We will follow up again with the folks in the SIG next week after we have time to review options.

Thank you @sttomm for bringing this to our attention. I think most of us not in CF world just ignored this because we did not understand the usage of it... our bad.

@duglin
Copy link
Contributor

duglin commented Apr 3, 2018

See: #483

@mattmcneeney
Copy link
Contributor

This issue still needs addressing alongside moving the dashboard_client field to the profile in #483

@mattmcneeney
Copy link
Contributor

This field does need to be globally unique for CF. Since this field has now been moved to the CF Catalog Extensions section, I think we can close this now.

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

No branches or pull requests

5 participants