scoped tokens #8393

deads2k · 2016-04-06T20:05:51Z

This demonstrates how scopes could be attached to a user token and then filtered by an authorizer that wraps our existing authorizer. It creates:

special:whoami - call users/~ to prove identity
special:access-check - call self-SARs to check access of the user (not the token)
clusterrole:<clusterrole name>:<namespace to allow the cluster role, * means all> - allows access based on cluster role rules, but only within the specified namespace. (I might make that a comma-delimited list.)

If we like the general approach, I can finish off the list:

unit tests
integration tests that create scoped tokens

And do the followups:

make SARs scope aware https://trello.com/c/o0pWuPmd/698-5-auth-scopes-scoped-access-checks-sars-tsars
make TSARs for checking a token https://trello.com/c/o0pWuPmd/698-5-auth-scopes-scoped-access-checks-sars-tsars
add the other two scope types (namespace-wide and cluster-wide explicit) https://trello.com/c/fjVkPysA/711-auth-scopes-add-scopes-for-local-roles-and-explicit-permissions
tighten clusterrole/role name validation to the set of characters allowed in scope names? may not be possible... failing validation on one role would prevent the entire policy from being persisted Won't do this, but the scope validation prevents people from using them in a scope. If you want a scope for it, rename your role.
expand scope evaluator interface to provide human descriptions for scopes (used in grant flow) Provide descriptions #8807
expand scope evaluator interface to provide validation (used when requesting scopes or validating a user-created token) Add scope validation in api #8805
prevent escalating resources from being included by default:
remove *, secrets, oauthtokens, oauthauthorizationtokens, etc Prevent escalating resource access by default #8818
decide how a scope that grants a role including escalating resources would be specified Prevent escalating resource access by default #8818

@pweil- @liggitt @smarterclayton
@jwforres @jimmidyson @stefwalter you guys knew people who were looking to get openshift API tokens to perform actions for a user, right?

smarterclayton · 2016-04-06T20:48:30Z

Aren't there scope names defined in oauth that we have to support as well?
Do we need aliases for them?

On Wed, Apr 6, 2016 at 4:05 PM, David Eads [email protected] wrote:

This demonstrates how scopes could be attached to a user and then filtered
by an authorizer that wraps our existing authorizer. It creates:

special:whoami - call users/~ to prove identity

special:access-check - call self-SARs to check access of the user
(not the token)

clusterrole::<namespace to allow the cluster
role, * means all> - allows access based on cluster role rules, but
only within the specified namespace. (I might make that a comma-delimited
list.)

If we like the general approach, I can finish off the list:

unit tests

integration tests that create scoped tokens

And do the followups:

make SARs scope aware

make TSARs for checking a token

add the other two scope types (namespace-wide and cluster-wide
explicit)

@pweil- https://github.com/pweil- @liggitt https://github.com/liggitt
@smarterclayton https://github.com/smarterclayton
@jwforres https://github.com/jwforres @jimmidyson
https://github.com/jimmidyson @stefwalter
https://github.com/stefwalter you guys knew people who were looking to

get openshift API tokens to perform actions for a user, right?

You can view, comment on, or merge this pull request online at:

#8393
Commit Summary

add scope authorizer

UPSTREAM: 23574: add user.Info.GetExtra

File Changes

M Godeps/_workspace/src/k8s.io/kubernetes/pkg/auth/user/user.go
https://github.com/openshift/origin/pull/8393/files#diff-0 (9)

M pkg/auth/group/group_adder.go
https://github.com/openshift/origin/pull/8393/files#diff-1 (1)

M pkg/auth/oauth/registry/tokenauthenticator.go
https://github.com/openshift/origin/pull/8393/files#diff-2 (3)

M pkg/authorization/authorizer/authorizer.go
https://github.com/openshift/origin/pull/8393/files#diff-3 (8)

A pkg/authorization/authorizer/scope/authorizer.go
https://github.com/openshift/origin/pull/8393/files#diff-4 (71)

A pkg/authorization/authorizer/scope/converter.go
https://github.com/openshift/origin/pull/8393/files#diff-5 (140)

M pkg/cmd/server/origin/master_config.go
https://github.com/openshift/origin/pull/8393/files#diff-6 (6)

Patch Links:

https://github.com/openshift/origin/pull/8393.patch

https://github.com/openshift/origin/pull/8393.diff

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#8393

smarterclayton · 2016-04-06T20:48:50Z

Also, Peter the Great is not amused.

On Wed, Apr 6, 2016 at 4:48 PM, Clayton Coleman [email protected] wrote:

Aren't there scope names defined in oauth that we have to support as
well? Do we need aliases for them?

On Wed, Apr 6, 2016 at 4:05 PM, David Eads [email protected]
wrote:

This demonstrates how scopes could be attached to a user and then
filtered by an authorizer that wraps our existing authorizer. It creates:

special:whoami - call users/~ to prove identity

special:access-check - call self-SARs to check access of the user
(not the token)

clusterrole::<namespace to allow the cluster
role, * means all> - allows access based on cluster role rules, but
only within the specified namespace. (I might make that a comma-delimited
list.)

If we like the general approach, I can finish off the list:

unit tests

integration tests that create scoped tokens

And do the followups:

make SARs scope aware

make TSARs for checking a token

add the other two scope types (namespace-wide and cluster-wide
explicit)

@pweil- https://github.com/pweil- @liggitt https://github.com/liggitt
@smarterclayton https://github.com/smarterclayton
@jwforres https://github.com/jwforres @jimmidyson
https://github.com/jimmidyson @stefwalter
https://github.com/stefwalter you guys knew people who were looking to

get openshift API tokens to perform actions for a user, right?

You can view, comment on, or merge this pull request online at:

#8393
Commit Summary

add scope authorizer

UPSTREAM: 23574: add user.Info.GetExtra

File Changes

M Godeps/_workspace/src/k8s.io/kubernetes/pkg/auth/user/user.go
https://github.com/openshift/origin/pull/8393/files#diff-0 (9)

M pkg/auth/group/group_adder.go
https://github.com/openshift/origin/pull/8393/files#diff-1 (1)

M pkg/auth/oauth/registry/tokenauthenticator.go
https://github.com/openshift/origin/pull/8393/files#diff-2 (3)

M pkg/authorization/authorizer/authorizer.go
https://github.com/openshift/origin/pull/8393/files#diff-3 (8)

A pkg/authorization/authorizer/scope/authorizer.go
https://github.com/openshift/origin/pull/8393/files#diff-4 (71)

A pkg/authorization/authorizer/scope/converter.go
https://github.com/openshift/origin/pull/8393/files#diff-5 (140)

M pkg/cmd/server/origin/master_config.go
https://github.com/openshift/origin/pull/8393/files#diff-6 (6)

Patch Links:

https://github.com/openshift/origin/pull/8393.patch

https://github.com/openshift/origin/pull/8393.diff

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#8393

liggitt · 2016-04-06T20:56:02Z

There are standard scopes in OIDC (https://openid.net/specs/openid-connect-basic-1_0.html#Scopes)

deads2k · 2016-04-06T22:00:11Z

I'll need to reread the openid bits. I thought those were used by a client to request certain claims be provided. The user was still asked, but the token held the information, not any lasting access. If the access does have to be provided, chances are we need new endpoints to match the spec format, right?

stefwalter · 2016-04-07T08:14:08Z

pkg/authorization/authorizer/scope/authorizer.go

+func (a *scopeAuthorizer) Authorize(ctx kapi.Context, passedAttributes defaultauthorizer.AuthorizationAttributes) (bool, string, error) {
+	user, exists := kapi.UserFrom(ctx)
+	if !exists {
+		return false, "", fmt.Errorf("no user")


Suggest a more descriptive error message here if there's any risk of it ending up at an API caller.

stefwalter · 2016-04-07T08:15:29Z

@deads2k The main use case that registry users needed is a long lived token that they can plug into 'docker login'. That is, not have to run a docker login command with a new token every 24 hours.

Would we need the registry container need to be modified to do a special access check?

CC @aweiteka I believe this is about the long lived tokens you wanted earlier.

deads2k · 2016-04-07T11:40:38Z

Would we need the registry container need to be modified to do a special access check?

This would allow a user to provide a token that is only good for manipulating images instead of trusting the registry with all their access, including things like modifying group membership as an example. This is also something we'd want for a user created token flow, but doesn't provide one in and of itself.

deads2k · 2016-04-07T12:58:07Z

There are standard scopes in OIDC (https://openid.net/specs/openid-connect-basic-1_0.html#Scopes)

Yeah, we don't need to add those yet. This structure doesn't preclude them from working, but for now we aren't close to being an openID provider. If we want to be, that should be a separate objective.

aweiteka · 2016-04-07T14:54:37Z

This would allow a user to provide a token that is only good for manipulating images

The docker push/pull API commands are ultimately what I'm looking for supporting so this subset of permissions or access control sounds like it could satisfy that.

deads2k · 2016-04-26T19:51:28Z

@openshift/api-review this is on my next list. Do we like the shape of these? It's basically, <category>:<category-specific-format>, which initial support for curated scopes and cluster-role scopes applied to a namespace.

smarterclayton · 2016-04-26T21:18:41Z

If we supported OIDC in the future - those would be category-less?

smarterclayton · 2016-04-26T21:20:00Z

pkg/authorization/authorizer/scope/converter.go

+
+	tokens := strings.SplitN(scope, ":", 2)
+	if len(tokens) != 2 {
+		return nil, fmt.Errorf("bad format for %v", scope)


"bad format for scope %v" so I can find it when I have to debug it

deads2k · 2016-04-27T15:21:47Z

comments addressed, upstream pick update, tests written.

This is ready for review.

@liggitt you or a delegating victim?

deads2k · 2016-04-28T16:31:59Z

[test]

brianwcook · 2016-04-28T20:20:12Z

what happens if you login using a long lived token when you have openid connect provider enabled?
You get a token and then the provider shuts off your account. Is this accounted for?

deads2k · 2016-04-28T20:34:04Z

what happens if you login using a long lived token when you have openid connect provider enabled?

The cluster-admin determines the maximum token lifespan, so they determine long-livedness. Right now, there is no re-authentication check required for the lifespan of the token. I think its 24 hours by default.

This pull doesn't make the problem any better or worse.

deads2k · 2016-04-29T03:42:49Z

@smarterclayton this blocks oauth exposure. How's the API?

smarterclayton · 2016-04-29T03:54:21Z

I've seen worse. Approved.

deads2k · 2016-04-29T11:58:54Z

I've seen worse. Approved.

I feel like I should find the movie review translating tool they use for commercials. :)

@pweil- got a reviewer in mind?

pweil- · 2016-04-29T12:47:40Z

@liggitt and @sgallagher PTAL and give a 👍 or 👎

soltysh · 2016-04-29T13:47:25Z

pkg/authorization/authorizer/scope/converter.go

+			// API discovery/negotiation
+			"/api", "/api/*",
+			"/apis", "/apis/*",
+			"/oapi", "/oapi/*",


Can we make constants out of it, there's a TODO in master.go to have that prefix being configurable.

Can we make constants out of it, there's a TODO in master.go to have that prefix being configurable.

done

soltysh · 2016-04-29T14:04:50Z

@deads2k with yours and @liggitt explanation yesterday during planning and some time with your code I was able to grasp the idea. Thanks 🍺

liggitt · 2016-05-03T21:01:47Z

pkg/authorization/authorizer/scope/converter.go

+	SpecialIndicator       = "special:"
+	ClusterRoleIndicator   = "clusterrole:"
+	ClusterWideIndicator   = "clusterwide:"
+	NamespaceWideIndicator = "namespace:"


need to make sure we limit ourselves to valid characters for scopes (these are fine, just pointing it out)

no quote, no backslash, no space

colon, alphanumeric characters, comma is ok (full list of allowed chars is %x21 / %x23-5B / %x5D-7E)

deads2k · 2016-05-04T19:54:00Z

So if I make a constant and eliminate the "special:" prefix (I still think we're likely to regret that some day. Unscoped things always cause problems eventually.), I'll get a merge?

liggitt · 2016-05-05T20:17:51Z

I'm ok with the approach. possible name adjustments:

specific scopes:

user:info
user:check-access

parameterized scopes:

role:<cluster role>:<effective namespace>
role:<cluster role>:*

eventually:

user:create-token
user:request-project
local-role:<role>:<namespace>

followups:

tighten clusterrole/role name validation to the set of characters allowed in scope names? may not be possible... failing validation on one role would prevent the entire policy from being persisted
expand scope evaluator interface to provide human descriptions for scopes (used in grant flow)
expand scope evaluator interface to provide validation (used when requesting scopes or validating a user-created token)
prevent escalating resources from being included by default:
- remove *, secrets, oauthtokens, oauthauthorizationtokens, etc
decide how a scope that grants a role including escalating resources would be specified

deads2k · 2016-05-05T20:29:54Z

I'm ok with the approach. possible name adjustments:

Ok, those look fine.

@sgallagher all comments so far are minor and I'll take care of them tomorrow. Do you have anything structural?

deads2k · 2016-05-09T18:01:26Z

comments address [merge]

openshift-bot · 2016-05-09T18:05:31Z

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_requests_origin/5860/) (Image: devenv-rhel7_4149)

openshift-bot · 2016-05-09T19:15:32Z

Evaluated for origin test up to a85a7c6

openshift-bot · 2016-05-09T20:20:33Z

continuous-integration/openshift-jenkins/test FAILURE (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/3704/)

openshift-bot · 2016-05-10T02:43:54Z

Evaluated for origin merge up to a85a7c6

stefwalter reviewed Apr 7, 2016
View reviewed changes

deads2k added the needs-api-review label Apr 26, 2016

smarterclayton reviewed Apr 26, 2016
View reviewed changes

deads2k force-pushed the scopes branch from 486eeba to fd1de8a Compare April 27, 2016 15:20

deads2k changed the title ~~[POC] scoped tokens~~ scoped tokens Apr 27, 2016

deads2k force-pushed the scopes branch from fd1de8a to e14416d Compare April 28, 2016 16:31

deads2k force-pushed the scopes branch from e14416d to 839cc2a Compare April 28, 2016 18:20

smarterclayton added api-approved and removed needs-api-review labels Apr 29, 2016

soltysh reviewed Apr 29, 2016
View reviewed changes

deads2k assigned sgallagher May 2, 2016

liggitt reviewed May 3, 2016
View reviewed changes

openshift-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 3, 2016

UPSTREAM: 23574: add user.Info.GetExtra

36ec10b

deads2k force-pushed the scopes branch from 839cc2a to 25425f6 Compare May 9, 2016 18:00

openshift-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 9, 2016

add scope authorizer

a85a7c6

deads2k force-pushed the scopes branch from 25425f6 to a85a7c6 Compare May 9, 2016 19:14

openshift-bot merged commit 25f92f3 into openshift:master May 10, 2016

deads2k deleted the scopes branch September 6, 2016 17:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

scoped tokens #8393

scoped tokens #8393

deads2k commented Apr 6, 2016 •

edited

Loading

smarterclayton commented Apr 6, 2016

get openshift API tokens to perform actions for a user, right?

smarterclayton commented Apr 6, 2016

get openshift API tokens to perform actions for a user, right?

liggitt commented Apr 6, 2016

deads2k commented Apr 6, 2016

stefwalter Apr 7, 2016

stefwalter commented Apr 7, 2016

deads2k commented Apr 7, 2016

deads2k commented Apr 7, 2016

aweiteka commented Apr 7, 2016

deads2k commented Apr 26, 2016

smarterclayton commented Apr 26, 2016

smarterclayton Apr 26, 2016

deads2k commented Apr 27, 2016

deads2k commented Apr 28, 2016

brianwcook commented Apr 28, 2016

deads2k commented Apr 28, 2016

deads2k commented Apr 29, 2016

smarterclayton commented Apr 29, 2016

deads2k commented Apr 29, 2016

pweil- commented Apr 29, 2016

soltysh Apr 29, 2016

deads2k May 9, 2016

soltysh commented Apr 29, 2016

liggitt May 3, 2016 •

edited

Loading

deads2k commented May 4, 2016

liggitt commented May 5, 2016 •

edited

Loading

deads2k commented May 5, 2016

deads2k commented May 9, 2016

openshift-bot commented May 9, 2016 •

edited

Loading

openshift-bot commented May 9, 2016

openshift-bot commented May 9, 2016

openshift-bot commented May 10, 2016

scoped tokens #8393

scoped tokens #8393

Conversation

deads2k commented Apr 6, 2016 • edited Loading

smarterclayton commented Apr 6, 2016

get openshift API tokens to perform actions for a user, right?

smarterclayton commented Apr 6, 2016

get openshift API tokens to perform actions for a user, right?

liggitt commented Apr 6, 2016

deads2k commented Apr 6, 2016

stefwalter Apr 7, 2016

Choose a reason for hiding this comment

stefwalter commented Apr 7, 2016

deads2k commented Apr 7, 2016

deads2k commented Apr 7, 2016

aweiteka commented Apr 7, 2016

deads2k commented Apr 26, 2016

smarterclayton commented Apr 26, 2016

smarterclayton Apr 26, 2016

Choose a reason for hiding this comment

deads2k commented Apr 27, 2016

deads2k commented Apr 28, 2016

brianwcook commented Apr 28, 2016

deads2k commented Apr 28, 2016

deads2k commented Apr 29, 2016

smarterclayton commented Apr 29, 2016

deads2k commented Apr 29, 2016

pweil- commented Apr 29, 2016

soltysh Apr 29, 2016

Choose a reason for hiding this comment

deads2k May 9, 2016

Choose a reason for hiding this comment

soltysh commented Apr 29, 2016

liggitt May 3, 2016 • edited Loading

Choose a reason for hiding this comment

deads2k commented May 4, 2016

liggitt commented May 5, 2016 • edited Loading

deads2k commented May 5, 2016

deads2k commented May 9, 2016

openshift-bot commented May 9, 2016 • edited Loading

openshift-bot commented May 9, 2016

openshift-bot commented May 9, 2016

openshift-bot commented May 10, 2016

deads2k commented Apr 6, 2016 •

edited

Loading

liggitt May 3, 2016 •

edited

Loading

liggitt commented May 5, 2016 •

edited

Loading

openshift-bot commented May 9, 2016 •

edited

Loading