Skip to content

use the upstream authorization filters #16110

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.

Sign up for GitHub

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

Merged
openshift-merge-robot merged 4 commits into from
Sep 6, 2017
Merged

use the upstream authorization filters #16110

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d9643ce
UPSTREAM: 51803: make url parsing in apiserver configurable
deads2k Sep 1, 2017
1ad1b82
UPSTREAM: 51932: fix format of forbidden messages
deads2k Sep 5, 2017
689cdee
use stock requestinfo and authorization filters
deads2k Sep 1, 2017
5f83f0c
message tweaks for kube
deads2k Sep 1, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
message tweaks for kube
  • Loading branch information
@deads2k
deads2k committed Sep 5, 2017
commit 5f83f0c333117a3b0570db754c94a113b23fc1a4
1 change: 1 addition & 0 deletions pkg/cmd/server/kubernetes/master/master_config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ var expectedGroupPreferredVersions []string = []string{
"admissionregistration.k8s.io/v1alpha1",
"apps/v1beta1,authentication.k8s.io/v1",
"authorization.k8s.io/v1",
"authorization.openshift.io/v1",
"autoscaling/v1",
"batch/v1",
"certificates.k8s.io/v1beta1",
Expand Down
2 changes: 1 addition & 1 deletion test/cmd/authentication.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ os::cmd::expect_success_and_text "oc get user/~ --token='${allescalatingpowersto
os::cmd::expect_success "oc get secrets --token='${allescalatingpowerstoken}' -n '${project}'"
# scopes allow it, but authorization doesn't
os::cmd::try_until_failure "oc get secrets --token='${allescalatingpowerstoken}' -n default"
os::cmd::expect_failure_and_text "oc get secrets --token='${allescalatingpowerstoken}' -n default" 'cannot list secrets in project'
os::cmd::expect_failure_and_text "oc get secrets --token='${allescalatingpowerstoken}' -n default" 'cannot list secrets in the namespace'
os::cmd::expect_success_and_text "oc get projects --token='${allescalatingpowerstoken}'" "${project}"
os::cmd::expect_success_and_text "oc policy can-i --list --token='${allescalatingpowerstoken}' -n '${project}'" 'get.*pods'

Expand Down
8 changes: 4 additions & 4 deletions test/cmd/status.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,15 +44,15 @@ os::cmd::expect_success_and_text "oc login --server=${KUBERNETES_MASTER} --certi
os::cmd::expect_success_and_text 'oc status' "You don't have any projects. You can try to create a new project, by running"
os::cmd::expect_success_and_text 'oc status --all-namespaces' "Showing all projects on server"
# make sure `oc status` does not re-use the "no projects" message from `oc login` if -n is specified
os::cmd::expect_failure_and_text 'oc status -n forbidden' 'Error from server \(Forbidden\): User "test-user" cannot get project "forbidden"'
os::cmd::expect_failure_and_text 'oc status -n forbidden' 'Error from server \(Forbidden\): User "test-user" cannot get projects in the namespace "forbidden"'
Copy link
Copy Markdown
Contributor

@enj enj Sep 1, 2017

Choose a reason for hiding this comment

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

This is confusing, to say the least.

Copy link
Copy Markdown
Contributor Author

@deads2k deads2k Sep 2, 2017

Choose a reason for hiding this comment

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

This particular one is ugly. I'd rather take the ugly and try to make it prettier upstream. Having separate handling chains is causing us to keep and track bugs that are already fixed upstream.


# create a new project
os::cmd::expect_success "oc new-project project-bar --display-name='my project' --description='test project'"
os::cmd::expect_success_and_text "oc project" 'Using project "project-bar"'

# make sure `oc status` does not use "no projects" message if there is a project created
os::cmd::expect_success_and_text 'oc status' "In project my project \(project-bar\) on server"
os::cmd::expect_failure_and_text 'oc status -n forbidden' 'Error from server \(Forbidden\): User "test-user" cannot get project "forbidden"'
os::cmd::expect_failure_and_text 'oc status -n forbidden' 'Error from server \(Forbidden\): User "test-user" cannot get projects in the namespace "forbidden"'

# create a second project
os::cmd::expect_success "oc new-project project-bar-2 --display-name='my project 2' --description='test project 2'"
Expand All @@ -62,7 +62,7 @@ os::cmd::expect_success_and_text "oc project" 'Using project "project-bar-2"'
# message since `project-bar` still exists
os::cmd::expect_success_and_text "oc delete project project-bar-2" 'project "project-bar-2" deleted'
# the deletion is asynchronous and can take a while, so wait until we see the error
os::cmd::try_until_text "oc status" 'Error from server \(Forbidden\): User "test-user" cannot get project "project-bar-2"'
os::cmd::try_until_text "oc status" 'Error from server \(Forbidden\): User "test-user" cannot get projects in the namespace "project-bar-2"'

# delete "project-bar" and test that `oc status` still does not return the "no projects" message.
# Although we are deleting the last remaining project, the current context's namespace is still set
Expand All @@ -71,7 +71,7 @@ os::cmd::try_until_text "oc status" 'Error from server \(Forbidden\): User "test
os::cmd::expect_success "oc project project-bar"
os::cmd::expect_success "oc delete project project-bar"
# the deletion is asynchronous and can take a while, so wait until we see the error
os::cmd::try_until_text "oc status" 'Error from server \(Forbidden\): User "test-user" cannot get project "project-bar"'
os::cmd::try_until_text "oc status" 'Error from server \(Forbidden\): User "test-user" cannot get projects in the namespace "project-bar"'
os::cmd::try_until_not_text "oc get projects" "project-bar"
os::cmd::try_until_not_text "oc get projects" "project-bar-2"
os::cmd::expect_success "oc logout"
Expand Down
16 changes: 8 additions & 8 deletions test/integration/authorization_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1174,17 +1174,17 @@ func TestAuthorizationSubjectAccessReview(t *testing.T) {
localReview: askCanEdgarDeletePods,
kubeAuthInterface: haroldSARGetter,
kubeNamespace: "mallet-project",
err: `User "harold" cannot create localsubjectaccessreviews in project "mallet-project"`,
kubeErr: `User "harold" cannot create localsubjectaccessreviews.authorization.k8s.io in project "mallet-project"`,
err: `User "harold" cannot create localsubjectaccessreviews in the namespace "mallet-project"`,
kubeErr: `User "harold" cannot create localsubjectaccessreviews.authorization.k8s.io in the namespace "mallet-project"`,
}.run(t)
subjectAccessReviewTest{
description: "system:anonymous denied ability to run subject access review in project mallet-project",
localInterface: anonymousClient.LocalSubjectAccessReviews("mallet-project"),
localReview: askCanEdgarDeletePods,
kubeAuthInterface: anonymousSARGetter,
kubeNamespace: "mallet-project",
err: `User "system:anonymous" cannot create localsubjectaccessreviews in project "mallet-project"`,
kubeErr: `User "system:anonymous" cannot create localsubjectaccessreviews.authorization.k8s.io in project "mallet-project"`,
err: `User "system:anonymous" cannot create localsubjectaccessreviews in the namespace "mallet-project"`,
kubeErr: `User "system:anonymous" cannot create localsubjectaccessreviews.authorization.k8s.io in the namespace "mallet-project"`,
}.run(t)
// ensure message does not leak whether the namespace exists or not
subjectAccessReviewTest{
Expand All @@ -1193,17 +1193,17 @@ func TestAuthorizationSubjectAccessReview(t *testing.T) {
localReview: askCanEdgarDeletePods,
kubeAuthInterface: haroldSARGetter,
kubeNamespace: "nonexistent-project",
err: `User "harold" cannot create localsubjectaccessreviews in project "nonexistent-project"`,
kubeErr: `User "harold" cannot create localsubjectaccessreviews.authorization.k8s.io in project "nonexistent-project"`,
err: `User "harold" cannot create localsubjectaccessreviews in the namespace "nonexistent-project"`,
kubeErr: `User "harold" cannot create localsubjectaccessreviews.authorization.k8s.io in the namespace "nonexistent-project"`,
}.run(t)
subjectAccessReviewTest{
description: "system:anonymous denied ability to run subject access review in project nonexistent-project",
localInterface: anonymousClient.LocalSubjectAccessReviews("nonexistent-project"),
localReview: askCanEdgarDeletePods,
kubeAuthInterface: anonymousSARGetter,
kubeNamespace: "nonexistent-project",
err: `User "system:anonymous" cannot create localsubjectaccessreviews in project "nonexistent-project"`,
kubeErr: `User "system:anonymous" cannot create localsubjectaccessreviews.authorization.k8s.io in project "nonexistent-project"`,
err: `User "system:anonymous" cannot create localsubjectaccessreviews in the namespace "nonexistent-project"`,
kubeErr: `User "system:anonymous" cannot create localsubjectaccessreviews.authorization.k8s.io in the namespace "nonexistent-project"`,
}.run(t)

askCanHaroldUpdateProject := &authorizationapi.LocalSubjectAccessReview{
Expand Down
4 changes: 2 additions & 2 deletions test/integration/bootstrap_policy_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,8 @@ func TestBootstrapPolicySelfSubjectAccessReviews(t *testing.T) {
localReview: askCanClusterAdminsCreateProject,
kubeAuthInterface: valerieKubeClient.Authorization(),
kubeNamespace: "openshift",
err: `User "valerie" cannot create localsubjectaccessreviews in project "openshift"`,
kubeErr: `User "valerie" cannot create localsubjectaccessreviews.authorization.k8s.io in project "openshift"`,
err: `User "valerie" cannot create localsubjectaccessreviews in the namespace "openshift"`,
kubeErr: `User "valerie" cannot create localsubjectaccessreviews.authorization.k8s.io in the namespace "openshift"`,
}.run(t)

}
Expand Down
2 changes: 1 addition & 1 deletion test/integration/oauth_cert_fallback_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func TestOAuthCertFallback(t *testing.T) {
certUser = "system:admin"

unauthorizedError = "the server has asked for the client to provide credentials (get users ~)"
anonymousError = `User "system:anonymous" cannot get users at the cluster scope`
anonymousError = `User "system:anonymous" cannot get users at the cluster scope: User "system:anonymous" cannot get users at the cluster scope (get users ~)`
)

// Build master config
Expand Down