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

[#153072577] Add UAA client for paas-admin app #1135

Merged
merged 3 commits into from
Dec 14, 2017
Merged

Conversation

henrytk
Copy link
Contributor

@henrytk henrytk commented Dec 7, 2017

What

We need a UAA client configuration for the paas-admin app to use. It currently has limited permissions, because the app only lists orgs at the moment as proof the authentication works.

How to review

Review in conjunction with alphagov/paas-admin#1

There is little danger with this commit as the client is configured with much less scope than will eventually be required. This is on purpose.

Eyeball the code and deploy with passing tests should be enough.

Who can review

Anyone but me, @chrisfarms @camelpunch or @carolinegreen

@henrytk henrytk changed the title WIP [#153072577] Uaa auth 153072577 WIP [#153072577] Add UAA client for paas-admin app Dec 7, 2017
This client will eventually need more scopes for the various operations
a user might want to do via the app, but for now we limit it to OAuth
and read operations.
@henrytk henrytk force-pushed the uaa-auth-153072577 branch 3 times, most recently from 72c47eb to ada00c2 Compare December 7, 2017 16:37
@henrytk henrytk changed the title WIP [#153072577] Add UAA client for paas-admin app [#153072577] Add UAA client for paas-admin app Dec 7, 2017
@@ -136,6 +137,13 @@
)
}

it {
clients.each { |_, config|
expect(config.has_key?("override")).to be true
Copy link
Contributor

Choose a reason for hiding this comment

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

Because of rspec magic that translates have_xxx(foo) to has_xxx?(foo), you could also say expect(config).to have_key("override")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That reads more gooder. Thanks.

According to the UAA docs[1] this property allows the configuration
in the manifest to override client configurations saved if you have
a persistent database. We set this because we always want our manifest
to be the source of truth for configuration.

Also, we remove the `id` fields because they are redundant. The UAA
release uses the `properties.uaa.clients.*` key as the ID.

[1] https://github.com/cloudfoundry/uaa/blob/391163ebad397b8f3eb5298aa01412dd94c9a176/docs/Sysadmin-Guide.rst#clients
These are the groups that can be requested by the admin user when authenticating
via UAA.

Each UAA client is configured to allow requests for a set of scopes.
Currently, if the admin user authenticates via a UAA client with the `cloud_controller.admin_read_only`
scope it will not have that scope in the token.
@henrytk henrytk force-pushed the uaa-auth-153072577 branch from ada00c2 to 4e87fde Compare December 8, 2017 09:48
@dcarley dcarley merged commit 4e87fde into master Dec 14, 2017
dcarley added a commit that referenced this pull request Dec 14, 2017
[#153072577] Add UAA client for paas-admin app
@dcarley dcarley deleted the uaa-auth-153072577 branch December 14, 2017 12:28
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.

3 participants