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

oidc: Reimplement userinfo for fine-grained error handling #31

Merged
merged 5 commits into from
Jul 13, 2020

Conversation

yanniszark
Copy link
Contributor

The UserInfo function of the go-oidc library doesn't expose details
about the HTTP response from the OIDC Provider, if the request fails.
This makes it impossible to detect if the returned code is 401, in
which case the user's session at the AuthService should be revoked.

Initially, we were revoking the session on any error, but this caused
undesired revocations because of errors we hadn't thought of (context
cancellation).

Reimplement the call to UserInfo and expose HTTP response details to the
caller, so they can make a sensible decision.

Related: coreos/go-oidc#248

@yanniszark yanniszark force-pushed the feature-fix-oidc-userinfo-call branch 2 times, most recently from 5d71b3f to 305ef45 Compare July 12, 2020 20:31
apyrgio pushed a commit that referenced this pull request Jul 13, 2020
The `UserInfo` function of the `go-oidc` library doesn't expose details
about the HTTP response from the OIDC Provider, if the request fails.
This makes it impossible to detect if the returned code is `401`, in
which case the user's session at the AuthService should be revoked.

Initially, we were revoking the session on any error, but this caused
undesired revocations because of errors we hadn't thought of (context
cancellation).

Reimplement the call to UserInfo and expose HTTP response details to the
caller, so they can make a sensible decision.

Github-PR: #31

Signed-off-by: Yannis Zarkadas <[email protected]>
apyrgio pushed a commit that referenced this pull request Jul 13, 2020
The UserInfo endpoint is called with an OAuth2 access token. The access
token expires after a while and then we must renew it with the refresh
token. Update the code so that it takes care of renewing the access
token.

Closes #32
Github-PR: #31

Signed-off-by: Yannis Zarkadas <[email protected]>
apyrgio pushed a commit that referenced this pull request Jul 13, 2020
Having multiple kubeconfig files confuses k3d, which doesn't know what
kubeconfig file to use. Instruct it to always use the
`$HOME/.kube/config` file.

Github-PR: #31

Signed-off-by: Yannis Zarkadas <[email protected]>
apyrgio pushed a commit that referenced this pull request Jul 13, 2020
Add a unit test for the UserInfo function to make sure that we can get
back the request info when an error occurs.

Github-PR: #31

Signed-off-by: Yannis Zarkadas <[email protected]>
yanniszark added a commit that referenced this pull request Jul 13, 2020
The `UserInfo` function of the `go-oidc` library doesn't expose details
about the HTTP response from the OIDC Provider, if the request fails.
This makes it impossible to detect if the returned code is `401`, in
which case the user's session at the AuthService should be revoked.

Initially, we were revoking the session on any error, but this caused
undesired revocations because of errors we hadn't thought of (context
cancellation).

Reimplement the call to UserInfo and expose HTTP response details to the
caller, so they can make a sensible decision.

Github-PR: #31

Signed-off-by: Yannis Zarkadas <[email protected]>
yanniszark added a commit that referenced this pull request Jul 13, 2020
Having multiple kubeconfig files confuses k3d, which doesn't know what
kubeconfig file to use. Instruct it to always use the
`$HOME/.kube/config` file.

Github-PR: #31

Signed-off-by: Yannis Zarkadas <[email protected]>
yanniszark added a commit that referenced this pull request Jul 13, 2020
Add a unit test for the UserInfo function to make sure that we can get
back the request info when an error occurs.

Github-PR: #31

Signed-off-by: Yannis Zarkadas <[email protected]>
@yanniszark yanniszark force-pushed the feature-fix-oidc-userinfo-call branch from 305ef45 to a46cf71 Compare July 13, 2020 12:19
The `UserInfo` function of the `go-oidc` library doesn't expose details
about the HTTP response from the OIDC Provider, if the request fails.
This makes it impossible to detect if the returned code is `401`, in
which case the user's session at the AuthService should be revoked.

Initially, we were revoking the session on any error, but this caused
undesired revocations because of errors we hadn't thought of (context
cancellation).

Reimplement the call to UserInfo and expose HTTP response details to the
caller, so they can make a sensible decision.

Github-PR: #31

Signed-off-by: Yannis Zarkadas <[email protected]>
Having multiple kubeconfig files confuses k3d, which doesn't know what
kubeconfig file to use. Instruct it to always use the
`$HOME/.kube/config` file.

Github-PR: #31

Signed-off-by: Yannis Zarkadas <[email protected]>
Add a unit test for the UserInfo function to make sure that we can get
back the request info when an error occurs.

Github-PR: #31

Signed-off-by: Yannis Zarkadas <[email protected]>
Signed-off-by: Yannis Zarkadas <[email protected]>
@yanniszark yanniszark force-pushed the feature-fix-oidc-userinfo-call branch from a46cf71 to ac7a6ad Compare July 13, 2020 12:47
@yanniszark yanniszark force-pushed the feature-fix-oidc-userinfo-call branch from ac7a6ad to e48e130 Compare July 13, 2020 13:11
apyrgio pushed a commit that referenced this pull request Jul 13, 2020
The `UserInfo` function of the `go-oidc` library doesn't expose details
about the HTTP response from the OIDC Provider, if the request fails.
This makes it impossible to detect if the returned code is `401`, in
which case the user's session at the AuthService should be revoked.

Initially, we were revoking the session on any error, but this caused
undesired revocations because of errors we hadn't thought of (context
cancellation).

Reimplement the call to UserInfo and expose HTTP response details to the
caller, so they can make a sensible decision.

Github-PR: #31

Signed-off-by: Yannis Zarkadas <[email protected]>
Reviewed-by: Alex Pyrgiotis <[email protected]>
apyrgio pushed a commit that referenced this pull request Jul 13, 2020
Having multiple kubeconfig files confuses k3d, which doesn't know what
kubeconfig file to use. Instruct it to always use the
`$HOME/.kube/config` file.

Github-PR: #31

Signed-off-by: Yannis Zarkadas <[email protected]>
Reviewed-by: Alex Pyrgiotis <[email protected]>
apyrgio pushed a commit that referenced this pull request Jul 13, 2020
Add a unit test for the UserInfo function to make sure that we can get
back the request info when an error occurs.

Github-PR: #31

Signed-off-by: Yannis Zarkadas <[email protected]>
Reviewed-by: Alex Pyrgiotis <[email protected]>
apyrgio pushed a commit that referenced this pull request Jul 13, 2020
Github-PR: #31

Signed-off-by: Yannis Zarkadas <[email protected]>
Reviewed-by: Alex Pyrgiotis <[email protected]>
@apyrgio apyrgio merged commit d10906e into master Jul 13, 2020
@apyrgio apyrgio deleted the feature-fix-oidc-userinfo-call branch July 13, 2020 13:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants