Remove OIDC from code #5233
Remove OIDC from code #5233
Conversation
arielshaqed
added
area/auth
pkg/api/auth_middleware.go
Outdated
| @@ -2,10 +2,8 @@ package api | |||
|
|
|||
| import ( | |||
There was a problem hiding this comment.
not compiling unused resources
Note to releaser:Breaking change, requires minor version bump! |
| oidc_auth: | ||
| type: apiKey | ||
| in: cookie | ||
| name: oidc_auth_session |
There was a problem hiding this comment.
we need this, that's the cookie that is verified by lakeFS.
pkg/api/auth_middleware.go
Outdated
| @@ -117,13 +112,7 @@ func checkSecurityRequirements(r *http.Request, | |||
| continue | |||
| } | |||
| user, err = userByToken(ctx, logger, authService, token) | |||
| case "oidc_auth": | |||
There was a problem hiding this comment.
we need this + userFromOIDC
|
@arielshaqed I'll go over it later today, we need someone from VE to be on the reviewers list too (we had a decision back then that shared-ownership repositories changes should be reviewed by at least 1 participant from each team) |
There was a problem hiding this comment.
overall looks good, I have nothing to add on top of what @Isan-Rivkin commented.
I do think we should test a built image using this feature branch with our common use-cases and cloud operation.
arielshaqed
force-pushed
the
chore/5231-oidc-thanks-for-all-the-fish
branch
from
a1f4014 to
4c99834
Compare
|
Restored the missing code, thanks! A private image works on a manually-hacked cloud dev-env. I tested explicit logout as well as deleted OIDC auth cookie. PTAL. |
| oidc_auth: | ||
| type: apiKey | ||
| in: cookie | ||
| name: oidc_auth_session |
There was a problem hiding this comment.
@arielshaqed Looking good!
Please don't forget that this release dependent on a new helm/lakefs release so that the configurations will not break :)
Out of scope.
Interpret OIDC claims that an _external_ auth service may set on the browser.
arielshaqed
force-pushed
the
chore/5231-oidc-thanks-for-all-the-fish
branch
from
4c99834 to
2a39ca5
Compare
|
Rebased on trunk; will auto-merge once tests pass. |
Should have done it on #5233, doing it now. Sorry. But just as well, some of these changes are _not_ due to that PR :-/
Should have done it on #5233, doing it now. Sorry. But just as well, some of these changes are _not_ due to that PR :-/
Was deprecated... and it's gone!