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

Bug 1538732 - Adding ability for Subject Rules Review to do the correct check. #693

Merged
merged 1 commit into from Jan 26, 2018

Conversation

shawn-hurley
Copy link
Contributor

Describe what this PR does and why we need it:
Allows the Subject Rules Review to do the full check.

Changes proposed in this pull request

  • Pass groups and extras to Client for validation
  • Pull out openshift scopes from extras
  • Pass groups and scopes to SRR Spec

Which issue this PR fixes (This will close that issue when PR gets merged)
fixes bug 1538732

@openshift-ci-robot openshift-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Jan 26, 2018
@shawn-hurley shawn-hurley added bug needs-review 3.9 | release-1.1 Kubernetes 1.9 | Openshift 3.9 | Broker release-1.1 and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jan 26, 2018
@shawn-hurley
Copy link
Contributor Author

shawn-hurley commented Jan 26, 2018

How to test

For the first scenario create user X and group Y. Add user X to group Y and grant group Y admin role. The broker before would say the user does not have enough permissions but now will respect the group.

For the scopes test you have to manipulate the CURL request to send a kubernetes identity

example request:
curl -k -X PUT -H "Authorization: bearer $(oc whoami -t)" -H "Content-type: application/json" -H "Accept: application/json" -H "X-Broker-API-Originating-Identity: kubernetes eyJncm91cHMiOlsic3lzdGVtOmF1dGhlbnRpY2F0ZWQ6b2F1dGgiLCJzeXN0ZW06YXV0aGVudGljYXRlZCJdLCJleHRyYSI6eyJzY29wZXMuYXV0aG9yaXphdGlvbi5vcGVuc2hpZnQuaW8iOlsidXNlcjppbmZvIl19LCJ1aWQiOiIiLCJ1c2VybmFtZSI6ImRldmVsb3BlciJ9Cg==" -d "{\"service_id\":\"f6c4486b7fb0cdac4b58e193607f7011\",\"plan_id\":\"76b2bdf5381b809657c90350726595e5\",\"organization_guid\":\"8471152c-02a3-11e8-8acd-0ec5edf551fd\",\"space_guid\":\"8471152c-02a3-11e8-8acd-0ec5edf551fd\",\"parameters\":{\"mediawiki_admin_pass\":\"shawnhurley\",\"mediawiki_admin_user\":\"admin\",\"mediawiki_db_schema\":\"mediawiki\",\"mediawiki_site_lang\":\"en\",\"mediawiki_site_name\":\"MediaWiki\"},\"context\":{\"namespace\":\"default\",\"platform\":\"kubernetes\"}}" https://asb-1338-ansible-service-broker.192.168.37.1.nip.io/ansible-service-broker/v2/service_instances/67168b70-72c9-47d0-bb7c-8fcd1d00f19d?accepts_incomplete=true

The X-Broker-API-Originating-Identity: kubernetes is the piece that we care about that example should contain this as the user identity:
{"groups":["system:authenticated:oauth","system:authenticated"],"extra":{"scopes.authorization.openshift.io":["user:info"]},"uid":"","username":"developer"}

This user should be granted admin access, but using this token should limit their scope to just info about the user and not let them continue.

@eriknelson
Copy link
Contributor

VISIACK, need some more help with the backports.

@eriknelson eriknelson merged commit aba2f38 into openshift:master Jan 26, 2018
jianzhangbjz pushed a commit to jianzhangbjz/ansible-service-broker that referenced this pull request May 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.9 | release-1.1 Kubernetes 1.9 | Openshift 3.9 | Broker release-1.1 needs-review size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants