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

Use variable request_uri as redirect after auth #1164

Merged
merged 1 commit into from
Aug 17, 2017

Conversation

aledbf
Copy link
Member

@aledbf aledbf commented Aug 17, 2017

No description provided.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Aug 17, 2017
@k8s-reviewable
Copy link

This change is Reviewable

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 44.687% when pulling ca2733ca6a44c48d0ec89c3527f49d296d77273e on aledbf:oauth2-auth into edb1c69 on kubernetes:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.03%) to 44.661% when pulling 91d8adce2b4ff1248873f33aa8129b70ccc64df0 on aledbf:oauth2-auth into f1598b3 on kubernetes:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.01%) to 44.687% when pulling 5e08600 on aledbf:oauth2-auth into 015ef8d on kubernetes:master.

@aledbf aledbf merged commit fe33cd3 into kubernetes:master Aug 17, 2017
@aledbf aledbf deleted the oauth2-auth branch August 19, 2017 18:33
@aledbf
Copy link
Member Author

aledbf commented Aug 21, 2017

I believe the two lines above should allow to return a cookie from auth backend to the sign-in url.
Unfortunately, this doesn't work.

Please make sure you are using the $host variable. It's not possible to use cookies from another domain

This passes query string to the sing-in url. What if the sign-url already contains query string? Wouldn't there be multiple "?" marks?

This is fixed in master.

@xeor
Copy link
Contributor

xeor commented Sep 3, 2017

The ? breaks my setup using bitly/oauth2_proxy.
In the callback request, I can see the cookie being set correctly, but for some reason, the browser doesn't set it.

Example, the callbacks will be:

So, state will be abc:/oauth2/sign_in?rd=/ on .12, which is probably why it doesn't work..

Can't find the fix in master? Will we need to hold back to .13 if we use oauth_proxy (or similar)?

@georgecrawford
Copy link

I'm not sure what my issue is, but it looks related. With .12 or higher, I see this redirect loop:

1. https://MY_K8S_DASHBOARD_URL
    - 302 to https://MY_K8S_DASHBOARD_URL/oauth2/sign_in?rd=/

2. https://MY_K8S_DASHBOARD_URL/oauth2/sign_in?rd=/
    - displays "Sign in with a GitLab account" button, which loads:

3. https://MY_K8S_DASHBOARD_URL/oauth2/start?rd=%2Foauth2%2Fsign_in%3Frd%3D%2F
    - 302 to https://MY_GITLAB_URL/oauth/authorize?approval_prompt=force&client_id=GITLAB_CLIENT_ID&redirect_uri=https%3A%2F%2FMY_K8S_DASHBOARD_URL%2Foauth2%2Fcallback&response_type=code&scope=api&state=STATE_PARAM%3A%2Foauth2%2Fsign_in%3Frd%3D%2F

4. https://MY_GITLAB_URL/oauth/authorize?approval_prompt=force&client_id=GITLAB_CLIENT_ID&redirect_uri=https%3A%2F%2FMY_K8S_DASHBOARD_URL%2Foauth2%2Fcallback&response_type=code&scope=api&state=STATE_PARAM%3A%2Foauth2%2Fsign_in%3Frd%3D%2F
    - 302 to https://MY_K8S_DASHBOARD_URL/oauth2/callback?code=CODE_PARAM&state=STATE_PARAM%3A%2Foauth2%2Fsign_in%3Frd%3D%2F

5. https://MY_K8S_DASHBOARD_URL/oauth2/callback?code=CODE_PARAM&state=STATE_PARAM%3A%2Foauth2%2Fsign_in%3Frd%3D%2F
    - 302 to /oauth2/sign_in?rd=/
    - set-cookie:_oauth2_proxy=COOKIE_STRING; Path=/; Domain=MY_K8S_DASHBOARD_URL; Expires=Sat, 16 Sep 2017 11:01:31 GMT; HttpOnly; Secure

6. https://MY_K8S_DASHBOARD_URL/oauth2/sign_in?rd=/
    - 200 OK
    - page again shows the "Sign in with a GitLab account" button from step 2

.11 works fine.

@marcusramberg
Copy link

This change breaks for me too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants