Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

fix: redirect to login page on OIDC expiry instead of showing raw JSON#18271

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
blink-so wants to merge3 commits intomain
base:main
Choose a base branch
Loading
fromfix/oidc-expiry-redirect
Open
Show file tree
Hide file tree
Changes fromall commits
Commits
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletioncli/configssh.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -446,7 +446,7 @@ func (r *RootCmd) configSSH() *serpent.Command {

if !bytes.Equal(configRaw, configModified) {
sshDir := filepath.Dir(sshConfigFile)
if err := os.MkdirAll(sshDir,0700); err != nil {
if err := os.MkdirAll(sshDir,0o700); err != nil {
return xerrors.Errorf("failed to create directory %q: %w", sshDir, err)
}

Expand Down
8 changes: 5 additions & 3 deletionscli/configssh_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -207,7 +207,7 @@ func TestConfigSSH_MissingDirectory(t *testing.T) {
// Check that the directory has proper permissions (0700)
sshDirInfo, err := os.Stat(sshDir)
require.NoError(t, err)
require.Equal(t, os.FileMode(0700), sshDirInfo.Mode().Perm(), "directory should have 0700 permissions")
require.Equal(t, os.FileMode(0o700), sshDirInfo.Mode().Perm(), "directory should have 0700 permissions")
}

func TestConfigSSH_FileWriteAndOptionsFlow(t *testing.T) {
Expand DownExpand Up@@ -358,7 +358,8 @@ func TestConfigSSH_FileWriteAndOptionsFlow(t *testing.T) {
strings.Join([]string{
headerEnd,
"",
}, "\n")},
}, "\n"),
},
},
args: []string{"--ssh-option", "ForwardAgent=yes"},
matches: []match{
Expand All@@ -383,7 +384,8 @@ func TestConfigSSH_FileWriteAndOptionsFlow(t *testing.T) {
strings.Join([]string{
headerEnd,
"",
}, "\n")},
}, "\n"),
},
},
args: []string{"--ssh-option", "ForwardAgent=yes"},
matches: []match{
Expand Down
58 changes: 33 additions & 25 deletionscoderd/httpmw/oauth2.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -70,17 +70,17 @@ func ExtractOAuth2(config promoauth.OAuth2Config, client *http.Client, cookieCfg
errorURI := r.URL.Query().Get("error_uri")
if errorMsg != "" {
// Combine the errors into a single string if either is provided.
if errorDescription == "" && errorURI != "" {
errorDescription = fmt.Sprintf("error_uri: %s", errorURI)
} else if errorDescription != "" && errorURI != "" {
errorDescription = fmt.Sprintf("%s, error_uri: %s", errorDescription, errorURI)
fullErrorMsg := fmt.Sprintf("Authentication failed: %s", errorMsg)
if errorDescription != "" {
fullErrorMsg = fmt.Sprintf("Authentication failed: %s - %s", errorMsg, errorDescription)
}
errorMsg = fmt.Sprintf("Encountered error in oidc process: %s", errorMsg)
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
Message: errorMsg,
// This message might be blank. This is ok.
Detail: errorDescription,
})
if errorURI != "" {
fullErrorMsg += fmt.Sprintf(" (error_uri: %s)", errorURI)
}

// Instead of returning a raw JSON error, redirect to login page with error message
loginURL := "/login?message=" + url.QueryEscape(fullErrorMsg)
http.Redirect(rw, r, loginURL, http.StatusTemporaryRedirect)
return
}

Expand DownExpand Up@@ -138,23 +138,26 @@ func ExtractOAuth2(config promoauth.OAuth2Config, client *http.Client, cookieCfg
}

if state == "" {
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
Message: "State must be provided.",
})
// Redirect to login with error message for missing state
errorMsg := "Authentication failed: Invalid authentication state"
loginURL := "/login?message=" + url.QueryEscape(errorMsg)
http.Redirect(rw, r, loginURL, http.StatusTemporaryRedirect)
return
}

stateCookie, err := r.Cookie(codersdk.OAuth2StateCookie)
if err != nil {
httpapi.Write(ctx, rw, http.StatusUnauthorized, codersdk.Response{
Message: fmt.Sprintf("Cookie %q must be provided.", codersdk.OAuth2StateCookie),
})
// Redirect to login with error message for missing cookie (likely expired session)
errorMsg := "Authentication failed: Session expired, please sign in again"
loginURL := "/login?message=" + url.QueryEscape(errorMsg)
http.Redirect(rw, r, loginURL, http.StatusTemporaryRedirect)
return
}
if stateCookie.Value != state {
httpapi.Write(ctx, rw, http.StatusUnauthorized, codersdk.Response{
Message: "State mismatched.",
})
// Redirect to login with error message for state mismatch (likely expired session)
errorMsg := "Authentication failed: Session expired, please sign in again"
loginURL := "/login?message=" + url.QueryEscape(errorMsg)
http.Redirect(rw, r, loginURL, http.StatusTemporaryRedirect)
return
}

Expand All@@ -165,17 +168,22 @@ func ExtractOAuth2(config promoauth.OAuth2Config, client *http.Client, cookieCfg

oauthToken, err := config.Exchange(ctx, code)
if err != nil {
errorCode := http.StatusInternalServerError
detail := err.Error()
if detail == "authorization_pending" {
// In the device flow, the token may not be immediately
// available. This is expected, and the client will retry.
errorCode = http.StatusBadRequest
// For device flow, we still return JSON as this is expected by the client
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
Message: "Failed exchanging Oauth code.",
Detail: detail,
})
return
}
httpapi.Write(ctx, rw, errorCode, codersdk.Response{
Message: "Failed exchanging Oauth code.",
Detail: detail,
})

// For other OAuth exchange errors (like expired tokens), redirect to login with error
errorMsg := fmt.Sprintf("Authentication failed: %s", detail)
loginURL := "/login?message=" + url.QueryEscape(errorMsg)
http.Redirect(rw, r, loginURL, http.StatusTemporaryRedirect)
return
}

Expand Down
70 changes: 67 additions & 3 deletionscoderd/httpmw/oauth2_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -98,15 +98,17 @@ func TestOAuth2(t *testing.T) {
res := httptest.NewRecorder()
tp := newTestOAuth2Provider(t, oauth2.AccessTypeOffline)
httpmw.ExtractOAuth2(tp, nil, codersdk.HTTPCookieConfig{}, nil)(nil).ServeHTTP(res, req)
require.Equal(t, http.StatusBadRequest, res.Result().StatusCode)
// Now redirects to login instead of returning JSON error
require.Equal(t, http.StatusTemporaryRedirect, res.Result().StatusCode)
})
t.Run("NoStateCookie", func(t *testing.T) {
t.Parallel()
req := httptest.NewRequest("GET", "/?code=something&state=test", nil)
res := httptest.NewRecorder()
tp := newTestOAuth2Provider(t, oauth2.AccessTypeOffline)
httpmw.ExtractOAuth2(tp, nil, codersdk.HTTPCookieConfig{}, nil)(nil).ServeHTTP(res, req)
require.Equal(t, http.StatusUnauthorized, res.Result().StatusCode)
// Now redirects to login instead of returning JSON error
require.Equal(t, http.StatusTemporaryRedirect, res.Result().StatusCode)
})
t.Run("MismatchedState", func(t *testing.T) {
t.Parallel()
Expand All@@ -118,7 +120,8 @@ func TestOAuth2(t *testing.T) {
res := httptest.NewRecorder()
tp := newTestOAuth2Provider(t, oauth2.AccessTypeOffline)
httpmw.ExtractOAuth2(tp, nil, codersdk.HTTPCookieConfig{}, nil)(nil).ServeHTTP(res, req)
require.Equal(t, http.StatusUnauthorized, res.Result().StatusCode)
// Now redirects to login instead of returning JSON error
require.Equal(t, http.StatusTemporaryRedirect, res.Result().StatusCode)
})
t.Run("ExchangeCodeAndState", func(t *testing.T) {
t.Parallel()
Expand DownExpand Up@@ -173,4 +176,65 @@ func TestOAuth2(t *testing.T) {
}
require.True(t, found, "expected state cookie")
})

// Test our new redirect functionality for OIDC errors
t.Run("OIDCErrorRedirectsToLogin", func(t *testing.T) {
t.Parallel()
req := httptest.NewRequest("GET", "/?error=invalid_request&error_description=The+request+is+missing+a+required+parameter", nil)
res := httptest.NewRecorder()
tp := newTestOAuth2Provider(t, oauth2.AccessTypeOffline)
httpmw.ExtractOAuth2(tp, nil, codersdk.HTTPCookieConfig{}, nil)(nil).ServeHTTP(res, req)

// Should redirect instead of returning JSON error
require.Equal(t, http.StatusTemporaryRedirect, res.Result().StatusCode)
location := res.Header().Get("Location")
require.Contains(t, location, "/login?message=")
require.Contains(t, location, "Authentication+failed")
})

t.Run("NoStateRedirectsToLogin", func(t *testing.T) {
t.Parallel()
req := httptest.NewRequest("GET", "/?code=something", nil)
res := httptest.NewRecorder()
tp := newTestOAuth2Provider(t, oauth2.AccessTypeOffline)
httpmw.ExtractOAuth2(tp, nil, codersdk.HTTPCookieConfig{}, nil)(nil).ServeHTTP(res, req)

// Should redirect instead of returning JSON error
require.Equal(t, http.StatusTemporaryRedirect, res.Result().StatusCode)
location := res.Header().Get("Location")
require.Contains(t, location, "/login?message=")
require.Contains(t, location, "Authentication+failed")
})

t.Run("NoStateCookieRedirectsToLogin", func(t *testing.T) {
t.Parallel()
req := httptest.NewRequest("GET", "/?code=something&state=test", nil)
res := httptest.NewRecorder()
tp := newTestOAuth2Provider(t, oauth2.AccessTypeOffline)
httpmw.ExtractOAuth2(tp, nil, codersdk.HTTPCookieConfig{}, nil)(nil).ServeHTTP(res, req)

// Should redirect instead of returning JSON error
require.Equal(t, http.StatusTemporaryRedirect, res.Result().StatusCode)
location := res.Header().Get("Location")
require.Contains(t, location, "/login?message=")
require.Contains(t, location, "Session+expired")
})

t.Run("MismatchedStateRedirectsToLogin", func(t *testing.T) {
t.Parallel()
req := httptest.NewRequest("GET", "/?code=something&state=test", nil)
req.AddCookie(&http.Cookie{
Name: codersdk.OAuth2StateCookie,
Value: "mismatch",
})
res := httptest.NewRecorder()
tp := newTestOAuth2Provider(t, oauth2.AccessTypeOffline)
httpmw.ExtractOAuth2(tp, nil, codersdk.HTTPCookieConfig{}, nil)(nil).ServeHTTP(res, req)

// Should redirect instead of returning JSON error
require.Equal(t, http.StatusTemporaryRedirect, res.Result().StatusCode)
location := res.Header().Get("Location")
require.Contains(t, location, "/login?message=")
require.Contains(t, location, "Session+expired")
})
}
1 change: 0 additions & 1 deletioncoderd/notifications/utils_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -94,7 +94,6 @@ func (i *dispatchInterceptor) Dispatcher(payload types.MessagePayload, title, bo
}

retryable, err = deliveryFn(ctx, msgID)

if err != nil {
i.err.Add(1)
i.lastErr.Store(err)
Expand Down
Loading
[8]ページ先頭
©2009-2025 Movatter.jp