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.

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

Jump to bottom

v2http: use guest access in non-TLS mode #6077

Merged
merged 1 commit into from
Aug 1, 2016
Merged

v2http: use guest access in non-TLS mode #6077

merged 1 commit into from
Aug 1, 2016

Conversation

gyuho
Copy link
Contributor

@gyuho gyuho commented Aug 1, 2016 •
edited
Loading

xiang90
xiang90 reviewed Aug 1, 2016
View reviewed changes
etcdserver/api/v2http/client_auth.go
}
plog.Warningf("auth: no authorization provided, checking guest access")
Copy link
Contributor

Choose a reason for hiding this comment

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

remove this logging.

@gyuho gyuho force-pushed the auth-guest branch 2 times, most recently from 07a2cda to a037588 Compare August 1, 2016 18:38
xiang90
xiang90 reviewed Aug 1, 2016
View reviewed changes
e2e/ctl_v2_test.go
@@ -312,6 +312,30 @@ func TestCtlV2AuthWithCommonName(t *testing.T) {
}
}

func TestCtlV2AuthWithGuestRole(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I do not think this is the correct test.

We should set up a cluster with tls + client Auth checking. Then a client with cert that has no valid CN name should be recognized as guest.

@gyuho gyuho force-pushed the auth-guest branch 2 times, most recently from ad7c608 to 594652b Compare August 1, 2016 19:38
@gyuho
Copy link
Contributor Author

gyuho commented Aug 1, 2016 •
edited
Loading

@xiang90 Just added a test case to TestPrefixAccess to test hasKeyPrefixAccess. I think this is enough to test if empty auth header without client cert enabled correctly uses guess access. If we want the invalid common name test case, we need e2e test to enable client cert auth in etcd binary.

PTAL.

@xiang90
Copy link
Contributor

xiang90 commented Aug 1, 2016

lgtm

@gyuho gyuho merged commit 9836990 into etcd-io:master Aug 1, 2016
@gyuho gyuho deleted the auth-guest branch August 1, 2016 21:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@gyuho @xiang90