diff --git a/pkg/auth/oauth/handlers/grant.go b/pkg/auth/oauth/handlers/grant.go
index 20f41f057c57..4e42b41a3b0e 100644
--- a/pkg/auth/oauth/handlers/grant.go
+++ b/pkg/auth/oauth/handlers/grant.go
@@ -162,7 +162,7 @@ func (g *redirectGrant) GrantNeeded(user user.Info, grant *api.Grant, w http.Res
redirectURL.RawQuery = url.Values{
"then": {req.URL.String()},
"client_id": {grant.Client.GetId()},
- "scopes": {grant.Scope},
+ "scope": {grant.Scope},
"redirect_uri": {grant.RedirectURI},
}.Encode()
http.Redirect(w, req, redirectURL.String(), http.StatusFound)
diff --git a/pkg/auth/oauth/registry/registry_test.go b/pkg/auth/oauth/registry/registry_test.go
index c5b8c5f3b9fd..32b2d68df281 100644
--- a/pkg/auth/oauth/registry/registry_test.go
+++ b/pkg/auth/oauth/registry/registry_test.go
@@ -239,7 +239,7 @@ func TestRegistryAndServer(t *testing.T) {
ClientAuthorization: testCase.ClientAuth,
}
if testCase.ClientAuth == nil {
- grant.Err = apierrs.NewNotFound(oapi.Resource("OAuthClientAuthorization"), "test:test")
+ grant.GetErr = apierrs.NewNotFound(oapi.Resource("OAuthClientAuthorization"), "test:test")
}
storage := registrystorage.New(access, authorize, client, NewUserConversion())
config := osinserver.NewDefaultServerConfig()
diff --git a/pkg/auth/server/grant/grant.go b/pkg/auth/server/grant/grant.go
index 21a6de765fa6..f0a2a9b15cc3 100644
--- a/pkg/auth/server/grant/grant.go
+++ b/pkg/auth/server/grant/grant.go
@@ -2,13 +2,13 @@ package grant
import (
"fmt"
- "html/template"
"net/http"
"net/url"
"strings"
kapi "k8s.io/kubernetes/pkg/api"
"k8s.io/kubernetes/pkg/auth/user"
+ "k8s.io/kubernetes/pkg/serviceaccount"
utilruntime "k8s.io/kubernetes/pkg/util/runtime"
"github.com/golang/glog"
@@ -26,7 +26,7 @@ const (
csrfParam = "csrf"
clientIDParam = "client_id"
userNameParam = "user_name"
- scopesParam = "scopes"
+ scopeParam = "scope"
redirectURIParam = "redirect_uri"
approveParam = "approve"
@@ -42,24 +42,38 @@ type FormRenderer interface {
type Form struct {
Action string
Error string
- Values FormValues
+
+ ServiceAccountName string
+ ServiceAccountNamespace string
+
+ GrantedScopes interface{}
+
+ Names GrantFormFields
+ Values GrantFormFields
+}
+
+type GrantFormFields struct {
+ Then string
+ CSRF string
+ ClientID string
+ UserName string
+ Scopes interface{}
+ RedirectURI string
+ Approve string
+ Deny string
}
-type FormValues struct {
- Then string
- ThenParam string
- CSRF string
- CSRFParam string
- ClientID string
- ClientIDParam string
- UserName string
- UserNameParam string
- Scopes string
- ScopesParam string
- RedirectURI string
- RedirectURIParam string
- ApproveParam string
- DenyParam string
+type Scope struct {
+ // Name is the string included in the OAuth scope parameter
+ Name string
+ // Description is a human-readable description of the scope. May be empty.
+ Description string
+ // Warning is a human-readable warning about the scope. Typically used to scare the user about escalating permissions. May be empty.
+ Warning string
+ // Error is a human-readable error, typically around the validity of the scope. May be empty.
+ Error string
+ // Granted indicates whether the user has already granted this scope.
+ Granted bool
}
type Grant struct {
@@ -108,10 +122,10 @@ func (l *Grant) ServeHTTP(w http.ResponseWriter, req *http.Request) {
func (l *Grant) handleForm(user user.Info, w http.ResponseWriter, req *http.Request) {
q := req.URL.Query()
- then := q.Get("then")
- clientID := q.Get("client_id")
- scopes := q.Get("scopes")
- redirectURI := q.Get("redirect_uri")
+ then := q.Get(thenParam)
+ clientID := q.Get(clientIDParam)
+ scopes := scope.Split(q.Get(scopeParam))
+ redirectURI := q.Get(redirectURIParam)
client, err := l.clientregistry.GetClient(kapi.NewContext(), clientID)
if err != nil || client == nil {
@@ -119,7 +133,7 @@ func (l *Grant) handleForm(user user.Info, w http.ResponseWriter, req *http.Requ
return
}
- if err := scopeauthorizer.ValidateScopeRestrictions(client, scope.Split(scopes)...); err != nil {
+ if err := scopeauthorizer.ValidateScopeRestrictions(client, scopes...); err != nil {
failure := fmt.Sprintf("%v requested illegal scopes (%v): %v", client.Name, scopes, err)
l.failed(failure, w, req)
return
@@ -139,41 +153,73 @@ func (l *Grant) handleForm(user user.Info, w http.ResponseWriter, req *http.Requ
return
}
+ grantedScopeNames := []string{}
+ grantedScopes := []Scope{}
+ requestedScopes := []Scope{}
+
+ clientAuthID := l.authregistry.ClientAuthorizationName(user.GetName(), client.Name)
+ if clientAuth, err := l.authregistry.GetClientAuthorization(kapi.NewContext(), clientAuthID); err == nil {
+ grantedScopeNames = clientAuth.Scopes
+ }
+
+ for _, s := range scopes {
+ requestedScopes = append(requestedScopes, getScopeData(s, grantedScopeNames))
+ }
+ for _, s := range grantedScopeNames {
+ grantedScopes = append(grantedScopes, getScopeData(s, grantedScopeNames))
+ }
+
form := Form{
- Action: uri.String(),
- Values: FormValues{
- Then: then,
- ThenParam: thenParam,
- CSRF: csrf,
- CSRFParam: csrfParam,
- ClientID: client.Name,
- ClientIDParam: clientIDParam,
- UserName: user.GetName(),
- UserNameParam: userNameParam,
- Scopes: scopes,
- ScopesParam: scopesParam,
- RedirectURI: redirectURI,
- RedirectURIParam: redirectURIParam,
- ApproveParam: approveParam,
- DenyParam: denyParam,
+ Action: uri.String(),
+ GrantedScopes: grantedScopes,
+ Names: GrantFormFields{
+ Then: thenParam,
+ CSRF: csrfParam,
+ ClientID: clientIDParam,
+ UserName: userNameParam,
+ Scopes: scopeParam,
+ RedirectURI: redirectURIParam,
+ Approve: approveParam,
+ Deny: denyParam,
+ },
+ Values: GrantFormFields{
+ Then: then,
+ CSRF: csrf,
+ ClientID: client.Name,
+ UserName: user.GetName(),
+ Scopes: requestedScopes,
+ RedirectURI: redirectURI,
},
}
+ if saNamespace, saName, err := serviceaccount.SplitUsername(client.Name); err == nil {
+ form.ServiceAccountName = saName
+ form.ServiceAccountNamespace = saNamespace
+ }
+
l.render.Render(form, w, req)
}
func (l *Grant) handleGrant(user user.Info, w http.ResponseWriter, req *http.Request) {
- if ok, err := l.csrf.Check(req, req.FormValue("csrf")); !ok || err != nil {
+ if ok, err := l.csrf.Check(req, req.FormValue(csrfParam)); !ok || err != nil {
glog.Errorf("Unable to check CSRF token: %v", err)
l.failed("Invalid CSRF token", w, req)
return
}
- then := req.FormValue("then")
- scopes := req.FormValue("scopes")
+ req.ParseForm()
+ then := req.FormValue(thenParam)
+ scopes := scope.Join(req.Form[scopeParam])
+ username := req.FormValue(userNameParam)
+
+ if username != user.GetName() {
+ glog.Errorf("User (%v) did not match authenticated user (%v)", username, user.GetName())
+ l.failed("User did not match", w, req)
+ return
+ }
- if len(req.FormValue(approveParam)) == 0 {
- // Redirect with rejection param
+ if len(req.FormValue(approveParam)) == 0 || len(scopes) == 0 {
+ // Redirect with an error param
url, err := url.Parse(then)
if len(then) == 0 || err != nil {
l.failed("Access denied, but no redirect URL was specified", w, req)
@@ -186,7 +232,7 @@ func (l *Grant) handleGrant(user user.Info, w http.ResponseWriter, req *http.Req
return
}
- clientID := req.FormValue("client_id")
+ clientID := req.FormValue(clientIDParam)
client, err := l.clientregistry.GetClient(kapi.NewContext(), clientID)
if err != nil || client == nil {
l.failed("Could not find client for client_id", w, req)
@@ -227,12 +273,16 @@ func (l *Grant) handleGrant(user user.Info, w http.ResponseWriter, req *http.Req
}
}
- if len(then) == 0 {
- l.failed("Approval granted, but no redirect URL was specified", w, req)
+ // Redirect, overriding the scope param on the redirect with the scopes that were actually granted
+ url, err := url.Parse(then)
+ if len(then) == 0 || err != nil {
+ l.failed("Access granted, but no redirect URL was specified", w, req)
return
}
-
- http.Redirect(w, req, then, http.StatusFound)
+ q := url.Query()
+ q.Set("scope", scopes)
+ url.RawQuery = q.Encode()
+ http.Redirect(w, req, url.String(), http.StatusFound)
}
func (l *Grant) failed(reason string, w http.ResponseWriter, req *http.Request) {
@@ -261,6 +311,29 @@ func getBaseURL(req *http.Request) (*url.URL, error) {
return uri, nil
}
+func getScopeData(scopeName string, grantedScopeNames []string) Scope {
+ scopeData := Scope{
+ Name: scopeName,
+ Error: fmt.Sprintf("Unknown scope"),
+ Granted: scope.Covers(grantedScopeNames, []string{scopeName}),
+ }
+ for _, evaluator := range scopeauthorizer.ScopeEvaluators {
+ if !evaluator.Handles(scopeName) {
+ continue
+ }
+ description, warning, err := evaluator.Describe(scopeName)
+ scopeData.Description = description
+ scopeData.Warning = warning
+ if err == nil {
+ scopeData.Error = ""
+ } else {
+ scopeData.Error = err.Error()
+ }
+ break
+ }
+ return scopeData
+}
+
// DefaultFormRenderer displays a page prompting the user to approve an OAuth grant.
// The requesting client id, requested scopes, and redirect URI are displayed to the user.
var DefaultFormRenderer = grantTemplateRenderer{}
@@ -270,40 +343,8 @@ type grantTemplateRenderer struct{}
func (r grantTemplateRenderer) Render(form Form, w http.ResponseWriter, req *http.Request) {
w.Header().Add("Content-Type", "text/html; charset=UTF-8")
w.WriteHeader(http.StatusOK)
- if err := grantTemplate.Execute(w, form); err != nil {
+
+ if err := defaultGrantTemplate.Execute(w, form); err != nil {
utilruntime.HandleError(fmt.Errorf("unable to render grant template: %v", err))
}
}
-
-// TODO: allow template to be read from an external file
-var grantTemplate = template.Must(template.New("grantForm").Parse(`
-
-{{ if .Error }}
-{{ .Error }}
-{{ else }}
-
-{{ end }}
-`))
diff --git a/pkg/auth/server/grant/grant_test.go b/pkg/auth/server/grant/grant_test.go
index 9055ec1fa991..391cab99f20e 100644
--- a/pkg/auth/server/grant/grant_test.go
+++ b/pkg/auth/server/grant/grant_test.go
@@ -11,6 +11,7 @@ import (
"testing"
kapi "k8s.io/kubernetes/pkg/api"
+ kapierrors "k8s.io/kubernetes/pkg/api/errors"
"k8s.io/kubernetes/pkg/auth/user"
knet "k8s.io/kubernetes/pkg/util/net"
@@ -50,7 +51,9 @@ func badClientRegistry(err error) *test.ClientRegistry {
}
func emptyAuthRegistry() *test.ClientAuthorizationRegistry {
- return &test.ClientAuthorizationRegistry{}
+ return &test.ClientAuthorizationRegistry{
+ GetErr: kapierrors.NewNotFound(oapi.Resource("oauthclientauthorizations"), "foo"),
+ }
}
func existingAuthRegistry(scopes []string) *test.ClientAuthorizationRegistry {
auth := oapi.OAuthClientAuthorization{
@@ -85,14 +88,36 @@ func TestGrant(t *testing.T) {
Auth: goodAuth("username"),
ClientRegistry: goodClientRegistry("myclient", []string{"myredirect"}, []string{"myscope1", "myscope2"}),
AuthRegistry: emptyAuthRegistry(),
- Path: "/grant?client_id=myclient&scopes=myscope1%20myscope2&redirect_uri=/myredirect&then=/authorize",
+ Path: "/grant?client_id=myclient&scope=myscope1%20myscope2&redirect_uri=/myredirect&then=/authorize",
ExpectStatusCode: 200,
ExpectContains: []string{
`action="/grant"`,
`name="csrf" value="test"`,
`name="client_id" value="myclient"`,
- `name="scopes" value="myscope1 myscope2"`,
+ `checked name="scope" value="myscope1"`,
+ `checked name="scope" value="myscope2"`,
+ `name="redirect_uri" value="/myredirect"`,
+ `name="then" value="/authorize"`,
+ },
+ },
+
+ "display form with existing scopes": {
+ CSRF: &csrf.FakeCSRF{Token: "test"},
+ Auth: goodAuth("username"),
+ ClientRegistry: goodClientRegistry("myclient", []string{"myredirect"}, []string{"newscope1", "newscope2", "existingscope1", "existingscope2"}),
+ AuthRegistry: existingAuthRegistry([]string{"existingscope1", "existingscope2"}),
+ Path: "/grant?client_id=myclient&scope=newscope1%20newscope2%20existingscope1%20existingscope2&redirect_uri=/myredirect&then=/authorize",
+
+ ExpectStatusCode: 200,
+ ExpectContains: []string{
+ `action="/grant"`,
+ `name="csrf" value="test"`,
+ `name="client_id" value="myclient"`,
+ `checked name="scope" value="newscope1"`,
+ `checked name="scope" value="newscope1"`,
+ `type="hidden" name="scope" value="existingscope1"`,
+ `type="hidden" name="scope" value="existingscope2"`,
`name="redirect_uri" value="/myredirect"`,
`name="then" value="/authorize"`,
},
@@ -142,7 +167,7 @@ func TestGrant(t *testing.T) {
Path: "/grant",
PostValues: url.Values{
"client_id": {"myclient"},
- "scopes": {"myscope1 myscope2"},
+ "scope": {"myscope1", "myscope2"},
"redirect_uri": {"/myredirect"},
"then": {"/authorize"},
"csrf": {"wrong"},
@@ -152,6 +177,25 @@ func TestGrant(t *testing.T) {
ExpectContains: []string{"CSRF"},
},
+ "error when POST fails user check": {
+ CSRF: &csrf.FakeCSRF{Token: "test"},
+ Auth: goodAuth("username"),
+ ClientRegistry: goodClientRegistry("myclient", []string{"myredirect"}, []string{"myscope1", "myscope2"}),
+ AuthRegistry: emptyAuthRegistry(),
+ Path: "/grant",
+ PostValues: url.Values{
+ "client_id": {"myclient"},
+ "scope": {"myscope1", "myscope2"},
+ "redirect_uri": {"/myredirect"},
+ "then": {"/authorize"},
+ "csrf": {"test"},
+ "user_name": {"wrong"},
+ },
+
+ ExpectStatusCode: 200,
+ ExpectContains: []string{"User did not match"},
+ },
+
"error displaying form with invalid client": {
CSRF: &csrf.FakeCSRF{Token: "test"},
Auth: goodAuth("username"),
@@ -172,10 +216,11 @@ func TestGrant(t *testing.T) {
PostValues: url.Values{
"approve": {"true"},
"client_id": {"myclient"},
- "scopes": {"myscope1 myscope2"},
+ "scope": {"myscope1", "myscope2"},
"redirect_uri": {"/myredirect"},
"then": {"/authorize"},
"csrf": {"test"},
+ "user_name": {"username"},
},
ExpectStatusCode: 200,
@@ -191,15 +236,16 @@ func TestGrant(t *testing.T) {
PostValues: url.Values{
"approve": {"true"},
"client_id": {"myclient"},
- "scopes": {"myscope1 myscope2"},
+ "scope": {"myscope1", "myscope2"},
"redirect_uri": {"/myredirect"},
"then": {"/authorize"},
"csrf": {"test"},
+ "user_name": {"username"},
},
ExpectStatusCode: 302,
ExpectCreatedAuthScopes: []string{"myscope1", "myscope2"},
- ExpectRedirect: "/authorize",
+ ExpectRedirect: "/authorize?scope=myscope1+myscope2",
},
"successful create grant without redirect": {
@@ -211,9 +257,10 @@ func TestGrant(t *testing.T) {
PostValues: url.Values{
"approve": {"true"},
"client_id": {"myclient"},
- "scopes": {"myscope1 myscope2"},
+ "scope": {"myscope1", "myscope2"},
"redirect_uri": {"/myredirect"},
"csrf": {"test"},
+ "user_name": {"username"},
},
ExpectStatusCode: 200,
@@ -233,15 +280,37 @@ func TestGrant(t *testing.T) {
PostValues: url.Values{
"approve": {"true"},
"client_id": {"myclient"},
- "scopes": {"myscope1 myscope2"},
+ "scope": {"myscope1", "myscope2"},
"redirect_uri": {"/myredirect"},
"then": {"/authorize"},
"csrf": {"test"},
+ "user_name": {"username"},
},
ExpectStatusCode: 302,
ExpectUpdatedAuthScopes: []string{"myscope1", "myscope2"},
- ExpectRedirect: "/authorize",
+ ExpectRedirect: "/authorize?scope=myscope1+myscope2",
+ },
+
+ "successful update grant with partial additional scopes": {
+ CSRF: &csrf.FakeCSRF{Token: "test"},
+ Auth: goodAuth("username"),
+ ClientRegistry: goodClientRegistry("myclient", []string{"myredirect"}, []string{"newscope1", "newscope2", "existingscope1", "existingscope2"}),
+ AuthRegistry: existingAuthRegistry([]string{"existingscope2", "existingscope1"}),
+ Path: "/grant",
+ PostValues: url.Values{
+ "approve": {"true"},
+ "client_id": {"myclient"},
+ "scope": {"newscope1", "existingscope1"},
+ "redirect_uri": {"/myredirect"},
+ "then": {"/authorize?scope=newscope1+newscope2+existingscope1"},
+ "csrf": {"test"},
+ "user_name": {"username"},
+ },
+
+ ExpectStatusCode: 302,
+ ExpectUpdatedAuthScopes: []string{"existingscope1", "existingscope2", "newscope1"},
+ ExpectRedirect: "/authorize?scope=newscope1+existingscope1",
},
"successful update grant with additional scopes": {
@@ -253,18 +322,19 @@ func TestGrant(t *testing.T) {
PostValues: url.Values{
"approve": {"true"},
"client_id": {"myclient"},
- "scopes": {"newscope1 existingscope1"},
+ "scope": {"newscope1", "existingscope1"},
"redirect_uri": {"/myredirect"},
"then": {"/authorize"},
"csrf": {"test"},
+ "user_name": {"username"},
},
ExpectStatusCode: 302,
ExpectUpdatedAuthScopes: []string{"existingscope1", "existingscope2", "newscope1"},
- ExpectRedirect: "/authorize",
+ ExpectRedirect: "/authorize?scope=newscope1+existingscope1",
},
- "successful reject grant": {
+ "successful reject grant via deny button": {
CSRF: &csrf.FakeCSRF{Token: "test"},
Auth: goodAuth("username"),
ClientRegistry: goodClientRegistry("myclient", []string{"myredirect"}, []string{"myscope1", "myscope2"}),
@@ -273,10 +343,31 @@ func TestGrant(t *testing.T) {
PostValues: url.Values{
"deny": {"true"},
"client_id": {"myclient"},
- "scopes": {"newscope1 existingscope1"},
+ "scope": {"newscope1", "existingscope1"},
+ "redirect_uri": {"/myredirect"},
+ "then": {"/authorize"},
+ "csrf": {"test"},
+ "user_name": {"username"},
+ },
+
+ ExpectStatusCode: 302,
+ ExpectRedirect: "/authorize?error=access_denied",
+ },
+
+ "successful reject grant via unchecking all requested scopes and approving": {
+ CSRF: &csrf.FakeCSRF{Token: "test"},
+ Auth: goodAuth("username"),
+ ClientRegistry: goodClientRegistry("myclient", []string{"myredirect"}, []string{"myscope1", "myscope2"}),
+ AuthRegistry: existingAuthRegistry([]string{"existingscope2", "existingscope1"}),
+ Path: "/grant",
+ PostValues: url.Values{
+ "approve": {"true"},
+ "client_id": {"myclient"},
+ // "scope": {"newscope1", "existingscope1"},
"redirect_uri": {"/myredirect"},
"then": {"/authorize"},
"csrf": {"test"},
+ "user_name": {"username"},
},
ExpectStatusCode: 302,
diff --git a/pkg/auth/server/grant/templates.go b/pkg/auth/server/grant/templates.go
new file mode 100644
index 000000000000..72d5a16be0ef
--- /dev/null
+++ b/pkg/auth/server/grant/templates.go
@@ -0,0 +1,168 @@
+package grant
+
+import "html/template"
+
+var defaultGrantTemplate = template.Must(template.New("defaultGrantForm").Parse(defaultGrantTemplateString))
+
+const defaultGrantTemplateString = `
+
+
+
+
+
+ Authorize
+ {{ if and .ServiceAccountName .ServiceAccountNamespace }}
+ service account {{ .ServiceAccountName }} in project {{ .ServiceAccountNamespace }}
+ {{ else }}
+ {{ .Values.ClientID }}
+ {{ end }}
+
+
+
+
+
+{{ define "scope" }}
+ {{ .Name }}
+ {{ if .Description }}{{ .Description }}
+{{ end -}}
+ {{ if .Warning }}{{ .Warning }}
+{{ end -}}
+ {{ if .Error }}{{ .Error }}
+{{ end -}}
+{{ end }}
+
+
+{{ if .Error }}
+{{ .Error }}
+{{ else }}
+
+{{ end }}
+
+
+`
diff --git a/pkg/authorization/authorizer/scope/converter.go b/pkg/authorization/authorizer/scope/converter.go
index 5dbbbbb9059c..07ad842d59d0 100644
--- a/pkg/authorization/authorizer/scope/converter.go
+++ b/pkg/authorization/authorizer/scope/converter.go
@@ -85,17 +85,19 @@ func ScopesToVisibleNamespaces(scopes []string, clusterPolicyGetter client.Clust
}
const (
- UserIndicator = "user:"
- ClusterRoleIndicator = "role:"
- ClusterWideIndicator = "clusterwide:"
- NamespaceWideIndicator = "namespace:"
+ UserIndicator = "user:"
+ ClusterRoleIndicator = "role:"
)
// ScopeEvaluator takes a scope and returns the rules that express it
type ScopeEvaluator interface {
+ // Handles returns true if this evaluator can evaluate this scope
Handles(scope string) bool
- Describe(scope string) string
+ // Validate returns an error if the scope is malformed
Validate(scope string) error
+ // Describe returns a description, warning (typically used to warn about escalation dangers), or an error if the scope is malformed
+ Describe(scope string) (description string, warning string, err error)
+ // ResolveRules returns the policy rules that this scope allows
ResolveRules(scope, namespace string, clusterPolicyGetter client.ClusterPolicyLister) ([]authorizationapi.PolicyRule, error)
ResolveGettableNamespaces(scope string, clusterPolicyGetter client.ClusterPolicyLister) ([]string, error)
}
@@ -143,18 +145,18 @@ func (userEvaluator) Validate(scope string) error {
return fmt.Errorf("unrecognized scope: %v", scope)
}
-func (userEvaluator) Describe(scope string) string {
+func (userEvaluator) Describe(scope string) (string, string, error) {
switch scope {
case UserInfo:
- return "Information about you, including username, identity names, and group membership."
+ return "Read-only access to your user information (including username, identities, and group membership)", "", nil
case UserAccessCheck:
- return `Information about user privileges, e.g. "Can I create builds?"`
+ return `Read-only access to view your privileges (for example, "can I create builds?")`, "", nil
case UserListProject:
- return `See projects you're aware of and the metadata (display name, description, etc) about those projects.`
+ return `Read-only access to list your projects and view their metadata (display name, description, etc.)`, "", nil
case UserFull:
- return `Full access to the server with all of your permissions.`
+ return `Full read/write access with all of your permissions`, `Includes any access you have to escalating resources like secrets`, nil
default:
- return fmt.Sprintf("unrecognized scope: %v", scope)
+ return "", "", fmt.Errorf("unrecognized scope: %v", scope)
}
}
@@ -248,21 +250,32 @@ func ParseClusterRoleScope(scope string) (string /*role name*/, string /*namespa
return tokens[1][0:lastColonIndex], tokens[1][lastColonIndex+1:], escalating, nil
}
-func (e clusterRoleEvaluator) Describe(scope string) string {
+func (e clusterRoleEvaluator) Describe(scope string) (string, string, error) {
roleName, scopeNamespace, escalating, err := e.parseScope(scope)
if err != nil {
- return err.Error()
- }
- escalatingPhrase := "including any escalating resources like secrets"
- if !escalating {
- escalatingPhrase = "excluding any escalating resources like secrets"
+ return "", "", err
}
+ // Anything you can do [in project "foo" | server-wide] that is also allowed by the "admin" role[, except access escalating resources like secrets]
+
+ scopePhrase := ""
if scopeNamespace == authorizationapi.ScopesAllNamespaces {
- return roleName + " access in all projects, " + escalatingPhrase
+ scopePhrase = "server-wide"
+ } else {
+ scopePhrase = fmt.Sprintf("in project %q", scopeNamespace)
}
- return roleName + " access in the " + scopeNamespace + " project, " + escalatingPhrase
+ warning := ""
+ escalatingPhrase := ""
+ if escalating {
+ warning = fmt.Sprintf("Includes access to escalating resources like secrets")
+ } else {
+ escalatingPhrase = ", except access escalating resources like secrets"
+ }
+
+ description := fmt.Sprintf("Anything you can do %s that is also allowed by the %q role%s", scopePhrase, roleName, escalatingPhrase)
+
+ return description, warning, nil
}
func (e clusterRoleEvaluator) ResolveRules(scope, namespace string, clusterPolicyGetter client.ClusterPolicyLister) ([]authorizationapi.PolicyRule, error) {
diff --git a/pkg/oauth/registry/test/clientauthorization.go b/pkg/oauth/registry/test/clientauthorization.go
index e968f4b8da6e..64280be99184 100644
--- a/pkg/oauth/registry/test/clientauthorization.go
+++ b/pkg/oauth/registry/test/clientauthorization.go
@@ -9,11 +9,17 @@ import (
)
type ClientAuthorizationRegistry struct {
- Err error
- ClientAuthorizations *api.OAuthClientAuthorizationList
- ClientAuthorization *api.OAuthClientAuthorization
- CreatedAuthorization *api.OAuthClientAuthorization
- UpdatedAuthorization *api.OAuthClientAuthorization
+ GetErr error
+ ClientAuthorizations *api.OAuthClientAuthorizationList
+ ClientAuthorization *api.OAuthClientAuthorization
+
+ CreateErr error
+ CreatedAuthorization *api.OAuthClientAuthorization
+
+ UpdateErr error
+ UpdatedAuthorization *api.OAuthClientAuthorization
+
+ DeleteErr error
DeletedClientAuthorizationName string
}
@@ -22,24 +28,24 @@ func (r *ClientAuthorizationRegistry) ClientAuthorizationName(userName, clientNa
}
func (r *ClientAuthorizationRegistry) ListClientAuthorizations(ctx kapi.Context, options *kapi.ListOptions) (*api.OAuthClientAuthorizationList, error) {
- return r.ClientAuthorizations, r.Err
+ return r.ClientAuthorizations, r.GetErr
}
func (r *ClientAuthorizationRegistry) GetClientAuthorization(ctx kapi.Context, name string) (*api.OAuthClientAuthorization, error) {
- return r.ClientAuthorization, r.Err
+ return r.ClientAuthorization, r.GetErr
}
func (r *ClientAuthorizationRegistry) CreateClientAuthorization(ctx kapi.Context, grant *api.OAuthClientAuthorization) (*api.OAuthClientAuthorization, error) {
r.CreatedAuthorization = grant
- return r.ClientAuthorization, r.Err
+ return r.ClientAuthorization, r.CreateErr
}
func (r *ClientAuthorizationRegistry) UpdateClientAuthorization(ctx kapi.Context, grant *api.OAuthClientAuthorization) (*api.OAuthClientAuthorization, error) {
r.UpdatedAuthorization = grant
- return r.ClientAuthorization, r.Err
+ return r.ClientAuthorization, r.UpdateErr
}
func (r *ClientAuthorizationRegistry) DeleteClientAuthorization(ctx kapi.Context, name string) error {
r.DeletedClientAuthorizationName = name
- return r.Err
+ return r.DeleteErr
}
diff --git a/test/integration/sa_oauthclient_test.go b/test/integration/sa_oauthclient_test.go
index b0765b7fba95..da636f48423c 100644
--- a/test/integration/sa_oauthclient_test.go
+++ b/test/integration/sa_oauthclient_test.go
@@ -2,6 +2,7 @@ package integration
import (
"crypto/tls"
+ "fmt"
"net/http"
"net/http/cookiejar"
"net/http/httptest"
@@ -129,25 +130,6 @@ func TestSAAsOAuthClient(t *testing.T) {
t.Fatalf("unexpected error: %v", err)
}
- promptApprovalSuccess := []string{
- "GET /oauth/authorize",
- "received challenge",
- "GET /oauth/authorize",
- "redirect to /oauth/approve",
- "form",
- "POST /oauth/approve",
- "redirect to /oauth/authorize",
- "redirect to /oauthcallback",
- "code",
- }
- noPromptSuccess := []string{
- "GET /oauth/authorize",
- "received challenge",
- "GET /oauth/authorize",
- "redirect to /oauthcallback",
- "code",
- }
-
// Test with a normal OAuth client
{
oauthClientConfig := &osincli.ClientConfig{
@@ -161,7 +143,18 @@ func TestSAAsOAuthClient(t *testing.T) {
t.Log("Testing unrestricted scope")
oauthClientConfig.Scope = ""
// approval steps are needed for unscoped access
- runOAuthFlow(t, clusterAdminClientConfig, projectName, oauthClientConfig, authorizationCodes, authorizationErrors, true, true, promptApprovalSuccess)
+ runOAuthFlow(t, clusterAdminClientConfig, projectName, oauthClientConfig, nil, authorizationCodes, authorizationErrors, true, true, []string{
+ "GET /oauth/authorize",
+ "received challenge",
+ "GET /oauth/authorize",
+ "redirect to /oauth/approve",
+ "form",
+ "POST /oauth/approve",
+ "redirect to /oauth/authorize",
+ "redirect to /oauthcallback",
+ "code",
+ "scope:user:full",
+ })
// verify the persisted client authorization looks like we expect
if clientAuth, err := clusterAdminClient.OAuthClientAuthorizations().Get("harold:" + oauthClientConfig.ClientId); err != nil {
t.Fatalf("Unexpected error: %v", err)
@@ -175,20 +168,90 @@ func TestSAAsOAuthClient(t *testing.T) {
}
}
// approval steps are needed again for unscoped access
- runOAuthFlow(t, clusterAdminClientConfig, projectName, oauthClientConfig, authorizationCodes, authorizationErrors, true, true, promptApprovalSuccess)
+ runOAuthFlow(t, clusterAdminClientConfig, projectName, oauthClientConfig, nil, authorizationCodes, authorizationErrors, true, true, []string{
+ "GET /oauth/authorize",
+ "received challenge",
+ "GET /oauth/authorize",
+ "redirect to /oauth/approve",
+ "form",
+ "POST /oauth/approve",
+ "redirect to /oauth/authorize",
+ "redirect to /oauthcallback",
+ "code",
+ "scope:user:full",
+ })
// with the authorization stored, approval steps are skipped
- runOAuthFlow(t, clusterAdminClientConfig, projectName, oauthClientConfig, authorizationCodes, authorizationErrors, true, true, noPromptSuccess)
+ runOAuthFlow(t, clusterAdminClientConfig, projectName, oauthClientConfig, nil, authorizationCodes, authorizationErrors, true, true, []string{
+ "GET /oauth/authorize",
+ "received challenge",
+ "GET /oauth/authorize",
+ "redirect to /oauthcallback",
+ "code",
+ "scope:user:full",
+ })
- // Approval step is needed again. Second time, the approval steps is not needed
+ // Approval step is needed again
t.Log("Testing restricted scope")
- oauthClientConfig.Scope = "user:info"
- runOAuthFlow(t, clusterAdminClientConfig, projectName, oauthClientConfig, authorizationCodes, authorizationErrors, true, false, promptApprovalSuccess)
- runOAuthFlow(t, clusterAdminClientConfig, projectName, oauthClientConfig, authorizationCodes, authorizationErrors, true, false, noPromptSuccess)
+ oauthClientConfig.Scope = "user:info user:check-access"
+ // filter to disapprove of granting the user:check-access scope
+ deniedScope := false
+ inputFilter := func(inputType, name, value string) bool {
+ if inputType == "checkbox" && name == "scope" && value == "user:check-access" {
+ deniedScope = true
+ return false
+ }
+ return true
+ }
+ // our token only gets the approved one
+ runOAuthFlow(t, clusterAdminClientConfig, projectName, oauthClientConfig, inputFilter, authorizationCodes, authorizationErrors, true, false, []string{
+ "GET /oauth/authorize",
+ "received challenge",
+ "GET /oauth/authorize",
+ "redirect to /oauth/approve",
+ "form",
+ "POST /oauth/approve",
+ "redirect to /oauth/authorize",
+ "redirect to /oauthcallback",
+ "code",
+ "scope:user:info",
+ })
+ if !deniedScope {
+ t.Errorf("Expected form filter to deny user:info scope")
+ }
+ // second time, we approve all, and our token gets all requested scopes
+ runOAuthFlow(t, clusterAdminClientConfig, projectName, oauthClientConfig, nil, authorizationCodes, authorizationErrors, true, false, []string{
+ "GET /oauth/authorize",
+ "received challenge",
+ "GET /oauth/authorize",
+ "redirect to /oauth/approve",
+ "form",
+ "POST /oauth/approve",
+ "redirect to /oauth/authorize",
+ "redirect to /oauthcallback",
+ "code",
+ "scope:" + oauthClientConfig.Scope,
+ })
+ // third time, the approval steps is not needed, and the token gets all requested scopes
+ runOAuthFlow(t, clusterAdminClientConfig, projectName, oauthClientConfig, nil, authorizationCodes, authorizationErrors, true, false, []string{
+ "GET /oauth/authorize",
+ "received challenge",
+ "GET /oauth/authorize",
+ "redirect to /oauthcallback",
+ "code",
+ "scope:" + oauthClientConfig.Scope,
+ })
// Now request an unscoped token again, and no approval should be needed
t.Log("Testing unrestricted scope")
oauthClientConfig.Scope = ""
- runOAuthFlow(t, clusterAdminClientConfig, projectName, oauthClientConfig, authorizationCodes, authorizationErrors, true, true, noPromptSuccess)
+ runOAuthFlow(t, clusterAdminClientConfig, projectName, oauthClientConfig, nil, authorizationCodes, authorizationErrors, true, true, []string{
+ "GET /oauth/authorize",
+ "received challenge",
+ "GET /oauth/authorize",
+ "redirect to /oauthcallback",
+ "code",
+ "scope:user:full",
+ })
clusterAdminClient.OAuthClientAuthorizations().Delete("harold:" + oauthClientConfig.ClientId)
}
@@ -206,8 +269,26 @@ func TestSAAsOAuthClient(t *testing.T) {
t.Log("Testing allowed scopes")
// First time, the approval steps are needed
// Second time, the approval steps are skipped
- runOAuthFlow(t, clusterAdminClientConfig, projectName, oauthClientConfig, authorizationCodes, authorizationErrors, true, true, promptApprovalSuccess)
- runOAuthFlow(t, clusterAdminClientConfig, projectName, oauthClientConfig, authorizationCodes, authorizationErrors, true, true, noPromptSuccess)
+ runOAuthFlow(t, clusterAdminClientConfig, projectName, oauthClientConfig, nil, authorizationCodes, authorizationErrors, true, true, []string{
+ "GET /oauth/authorize",
+ "received challenge",
+ "GET /oauth/authorize",
+ "redirect to /oauth/approve",
+ "form",
+ "POST /oauth/approve",
+ "redirect to /oauth/authorize",
+ "redirect to /oauthcallback",
+ "code",
+ "scope:" + oauthClientConfig.Scope,
+ })
+ runOAuthFlow(t, clusterAdminClientConfig, projectName, oauthClientConfig, nil, authorizationCodes, authorizationErrors, true, true, []string{
+ "GET /oauth/authorize",
+ "received challenge",
+ "GET /oauth/authorize",
+ "redirect to /oauthcallback",
+ "code",
+ "scope:" + oauthClientConfig.Scope,
+ })
clusterAdminClient.OAuthClientAuthorizations().Delete("harold:" + oauthClientConfig.ClientId)
}
@@ -222,7 +303,7 @@ func TestSAAsOAuthClient(t *testing.T) {
SendClientSecretInParams: true,
}
t.Log("Testing disallowed scopes")
- runOAuthFlow(t, clusterAdminClientConfig, projectName, oauthClientConfig, authorizationCodes, authorizationErrors, false, false, []string{
+ runOAuthFlow(t, clusterAdminClientConfig, projectName, oauthClientConfig, nil, authorizationCodes, authorizationErrors, false, false, []string{
"GET /oauth/authorize",
"received challenge",
"GET /oauth/authorize",
@@ -243,7 +324,7 @@ func TestSAAsOAuthClient(t *testing.T) {
Scope: scope.Join([]string{"unknown-scope"}),
SendClientSecretInParams: true,
}
- runOAuthFlow(t, clusterAdminClientConfig, projectName, oauthClientConfig, authorizationCodes, authorizationErrors, false, false, []string{
+ runOAuthFlow(t, clusterAdminClientConfig, projectName, oauthClientConfig, nil, authorizationCodes, authorizationErrors, false, false, []string{
"GET /oauth/authorize",
"received challenge",
"GET /oauth/authorize",
@@ -266,8 +347,26 @@ func TestSAAsOAuthClient(t *testing.T) {
}
// First time, the approval is needed
// Second time, the approval is skipped
- runOAuthFlow(t, clusterAdminClientConfig, projectName, oauthClientConfig, authorizationCodes, authorizationErrors, true, false, promptApprovalSuccess)
- runOAuthFlow(t, clusterAdminClientConfig, projectName, oauthClientConfig, authorizationCodes, authorizationErrors, true, false, noPromptSuccess)
+ runOAuthFlow(t, clusterAdminClientConfig, projectName, oauthClientConfig, nil, authorizationCodes, authorizationErrors, true, false, []string{
+ "GET /oauth/authorize",
+ "received challenge",
+ "GET /oauth/authorize",
+ "redirect to /oauth/approve",
+ "form",
+ "POST /oauth/approve",
+ "redirect to /oauth/authorize",
+ "redirect to /oauthcallback",
+ "code",
+ "scope:" + oauthClientConfig.Scope,
+ })
+ runOAuthFlow(t, clusterAdminClientConfig, projectName, oauthClientConfig, nil, authorizationCodes, authorizationErrors, true, false, []string{
+ "GET /oauth/authorize",
+ "received challenge",
+ "GET /oauth/authorize",
+ "redirect to /oauthcallback",
+ "code",
+ "scope:" + oauthClientConfig.Scope,
+ })
clusterAdminClient.OAuthClientAuthorizations().Delete("harold:" + oauthClientConfig.ClientId)
}
}
@@ -300,6 +399,7 @@ func runOAuthFlow(
clusterAdminClientConfig *restclient.Config,
projectName string,
oauthClientConfig *osincli.ClientConfig,
+ inputFilter htmlutil.InputFilterFunc,
authorizationCodes chan string,
authorizationErrors chan string,
expectGrantSuccess bool,
@@ -382,7 +482,7 @@ func runOAuthFlow(
if len(forms) == 0 {
break
}
- req, err = htmlutil.NewRequestFromForm(forms[0], currentURL)
+ req, err = htmlutil.NewRequestFromForm(forms[0], currentURL, inputFilter)
if err != nil {
t.Error(err)
return
@@ -400,10 +500,6 @@ func runOAuthFlow(
t.Error("didn't get a code or an error")
}
- if !reflect.DeepEqual(operations, expectOperations) {
- t.Errorf("Expected:\n%v\nGot\n%v", expectOperations, operations)
- }
-
if len(authorizationCode) > 0 {
accessRequest := oauthRuntimeClient.NewAccessRequest(osincli.AUTHORIZATION_CODE, &osincli.AuthorizeData{Code: authorizationCode})
accessData, err := accessRequest.GetToken()
@@ -411,6 +507,7 @@ func runOAuthFlow(
t.Errorf("unexpected error: %v", err)
return
}
+ operations = append(operations, fmt.Sprintf("scope:%v", accessData.ResponseData["scope"]))
whoamiConfig := clientcmd.AnonymousClientConfig(clusterAdminClientConfig)
whoamiConfig.BearerToken = accessData.AccessToken
@@ -440,4 +537,9 @@ func runOAuthFlow(
return
}
}
+
+ if !reflect.DeepEqual(operations, expectOperations) {
+ t.Errorf("Expected:\n%#v\nGot\n%#v", expectOperations, operations)
+ }
+
}
diff --git a/test/util/html/form.go b/test/util/html/form.go
index a58b47410cc4..8592dcf771f0 100644
--- a/test/util/html/form.go
+++ b/test/util/html/form.go
@@ -36,11 +36,15 @@ func GetAttr(element *html.Node, attrName string) (string, bool) {
return "", false
}
+type InputFilterFunc func(inputType, inputName, inputValue string) bool
+
// NewRequestFromForm builds a request that simulates submitting the given form. It honors:
// Form method (defaults to GET)
// Form action (defaults to currentURL if missing, or currentURL's scheme/host if server-relative)
-// values (only the first type="submit" input's value is included)
-func NewRequestFromForm(form *html.Node, currentURL *url.URL) (*http.Request, error) {
+// Input values from elements
+// * only the first type="submit" input's value is included
+// * only radio and checkbox inputs with the "checked" attribute are included
+func NewRequestFromForm(form *html.Node, currentURL *url.URL, filterFunc InputFilterFunc) (*http.Request, error) {
var (
reqMethod string
reqURL *url.URL
@@ -84,17 +88,28 @@ func NewRequestFromForm(form *html.Node, currentURL *url.URL) (*http.Request, er
for _, input := range GetElementsByTagName(form, "input") {
if name, ok := GetAttr(input, "name"); ok {
if value, ok := GetAttr(input, "value"); ok {
- // Check if this is a submit input
- if inputType, _ := GetAttr(input, "type"); inputType == "submit" {
- // Only add the value of the first one.
+ inputType, _ := GetAttr(input, "type")
+
+ // Allow skipping inputs
+ if filterFunc != nil && !filterFunc(inputType, name, value) {
+ continue
+ }
+
+ switch inputType {
+ case "submit":
+ // If this is a submit input, only add the value of the first one.
// We're simulating submitting the form.
- if addedSubmit {
- continue
+ if !addedSubmit {
+ formData.Add(name, value)
+ addedSubmit = true
+ }
+ case "radio", "checkbox":
+ if _, checked := GetAttr(input, "checked"); checked {
+ formData.Add(name, value)
}
- // Remember we've added a submit input
- addedSubmit = true
+ default:
+ formData.Add(name, value)
}
- formData.Add(name, value)
}
}
}