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

chore: support redirect on auth failure for echo routes #8196

Merged
merged 4 commits into from
Oct 23, 2023

Conversation

hamidzr
Copy link
Member

@hamidzr hamidzr commented Oct 18, 2023

Description

  • add a middleware that supports optional redirects on auth failures for given routes
  • use it to set up this behavior for proxied servers

somewhat similar to what we do with det proxies for tasks

// to authenticate incoming HTTP requests coming through proxies.
func processProxyAuthentication(c echo.Context) (done bool, err error) {
user, _, err := user.GetService().UserAndSessionFromRequest(c.Request())
if errors.Is(err, db.ErrNotFound) {
return true, redirectToLogin(c)
} else if err != nil {
return true, err
}
if !user.Active {
return true, redirectToLogin(c)
}

Test Plan

no changes are expected when the internal proxied_servers config is not used.
when a proxied server (aka a sidecar) is used the unauthenticated requests to those paths are expected to be redirected to the signin ui

Commentary (optional)

Checklist

  • Changes have been manually QA'd
  • User-facing API changes need the "User-facing API Change" label.
  • Release notes should be added as a separate file under docs/release-notes/.
    See Release Note for details.
  • Licenses should be included for new code which was copied and/or modified from any external code.

Ticket

@cla-bot cla-bot bot added the cla-signed label Oct 18, 2023
@netlify
Copy link

netlify bot commented Oct 18, 2023

Deploy Preview for determined-ui canceled.

Name Link
🔨 Latest commit 01b70fe
🔍 Latest deploy log https://app.netlify.com/sites/determined-ui/deploys/65306741b94a790008c80f0d

@hamidzr hamidzr marked this pull request as ready for review October 18, 2023 23:13
@hamidzr hamidzr requested a review from a team as a code owner October 18, 2023 23:13
Copy link
Contributor

@salonig23 salonig23 left a comment

Choose a reason for hiding this comment

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

LGTM!

@hamidzr hamidzr merged commit f61d250 into main Oct 23, 2023
70 of 83 checks passed
@hamidzr hamidzr deleted the hz-support-redirect-on-failure branch October 23, 2023 18:23
@dannysauer dannysauer added this to the 0.26.3 milestone Feb 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants