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

Commit7b06fc7

Browse files
authored
refactor: simplify OAuth2 authorization flow and use 302 redirects (#18923)
# Refactor OAuth2 Provider Authorization FlowThis PR refactors the OAuth2 provider authorization flow by:1. Removing the `authorizeMW` middleware and directly implementing its functionality in the `ShowAuthorizePage` handler2. Simplifying function signatures by removing unnecessary parameters: - Removed `db` parameter from `ShowAuthorizePage` - Removed `accessURL` parameter from `ProcessAuthorize`3. Changing the redirect status code in `ProcessAuthorize` from 307 (Temporary Redirect) to 302 (Found) to improve compatibility with external OAuth2 apps and browsers. (Technical explanation: we replied with a 307 to a POST request, thus the browser performs a redirect to that URL as a POST request, but we need it to be a GET request to be compatible. Thus, we use the 302 redirect so that browsers turn it into a GET request when redirecting back to the redirect_uri.)The changes maintain the same functionality while simplifying the code and improving compatibility with external systems.
1 parent071383b commit7b06fc7

File tree

4 files changed

+45
-98
lines changed

4 files changed

+45
-98
lines changed

‎coderd/coderdtest/oidctest/helper.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -132,14 +132,14 @@ func OAuth2GetCode(rawAuthURL string, doRequest func(req *http.Request) (*http.R
132132
return"",xerrors.Errorf("failed to create auth request: %w",err)
133133
}
134134

135-
expCode:=http.StatusTemporaryRedirect
136135
resp,err:=doRequest(r)
137136
iferr!=nil {
138137
return"",xerrors.Errorf("request: %w",err)
139138
}
140139
deferresp.Body.Close()
141140

142-
ifresp.StatusCode!=expCode {
141+
// Accept both 302 (Found) and 307 (Temporary Redirect) as valid OAuth2 redirects
142+
ifresp.StatusCode!=http.StatusFound&&resp.StatusCode!=http.StatusTemporaryRedirect {
143143
return"",codersdk.ReadBodyAsError(resp)
144144
}
145145

‎coderd/oauth2.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ func (api *API) deleteOAuth2ProviderAppSecret() http.HandlerFunc {
116116
// @Success 200 "Returns HTML authorization page"
117117
// @Router /oauth2/authorize [get]
118118
func (api*API)getOAuth2ProviderAppAuthorize() http.HandlerFunc {
119-
returnoauth2provider.ShowAuthorizePage(api.Database,api.AccessURL)
119+
returnoauth2provider.ShowAuthorizePage(api.AccessURL)
120120
}
121121

122122
// @Summary OAuth2 authorization request (POST - process authorization).
@@ -131,7 +131,7 @@ func (api *API) getOAuth2ProviderAppAuthorize() http.HandlerFunc {
131131
// @Success 302 "Returns redirect with authorization code"
132132
// @Router /oauth2/authorize [post]
133133
func (api*API)postOAuth2ProviderAppAuthorize() http.HandlerFunc {
134-
returnoauth2provider.ProcessAuthorize(api.Database,api.AccessURL)
134+
returnoauth2provider.ProcessAuthorize(api.Database)
135135
}
136136

137137
// @Summary OAuth2 token exchange.

‎coderd/oauth2provider/authorize.go

Lines changed: 41 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"github.com/coder/coder/v2/coderd/httpapi"
1717
"github.com/coder/coder/v2/coderd/httpmw"
1818
"github.com/coder/coder/v2/codersdk"
19+
"github.com/coder/coder/v2/site"
1920
)
2021

2122
typeauthorizeParamsstruct {
@@ -67,16 +68,46 @@ func extractAuthorizeParams(r *http.Request, callbackURL *url.URL) (authorizePar
6768
}
6869

6970
// ShowAuthorizePage handles GET /oauth2/authorize requests to display the HTML authorization page.
70-
// It uses authorizeMW which intercepts GET requests to show the authorization form.
71-
funcShowAuthorizePage(db database.Store,accessURL*url.URL) http.HandlerFunc {
72-
handler:=authorizeMW(accessURL)(ProcessAuthorize(db,accessURL))
73-
returnhandler.ServeHTTP
71+
funcShowAuthorizePage(accessURL*url.URL) http.HandlerFunc {
72+
returnfunc(rw http.ResponseWriter,r*http.Request) {
73+
app:=httpmw.OAuth2ProviderApp(r)
74+
ua:=httpmw.UserAuthorization(r.Context())
75+
76+
callbackURL,err:=url.Parse(app.CallbackURL)
77+
iferr!=nil {
78+
site.RenderStaticErrorPage(rw,r, site.ErrorPageData{Status:http.StatusInternalServerError,HideStatus:false,Title:"Internal Server Error",Description:err.Error(),RetryEnabled:false,DashboardURL:accessURL.String(),Warnings:nil})
79+
return
80+
}
81+
82+
params,validationErrs,err:=extractAuthorizeParams(r,callbackURL)
83+
iferr!=nil {
84+
errStr:=make([]string,len(validationErrs))
85+
fori,err:=rangevalidationErrs {
86+
errStr[i]=err.Detail
87+
}
88+
site.RenderStaticErrorPage(rw,r, site.ErrorPageData{Status:http.StatusBadRequest,HideStatus:false,Title:"Invalid Query Parameters",Description:"One or more query parameters are missing or invalid.",RetryEnabled:false,DashboardURL:accessURL.String(),Warnings:errStr})
89+
return
90+
}
91+
92+
cancel:=params.redirectURL
93+
cancelQuery:=params.redirectURL.Query()
94+
cancelQuery.Add("error","access_denied")
95+
cancel.RawQuery=cancelQuery.Encode()
96+
97+
site.RenderOAuthAllowPage(rw,r, site.RenderOAuthAllowData{
98+
AppIcon:app.Icon,
99+
AppName:app.Name,
100+
CancelURI:cancel.String(),
101+
RedirectURI:r.URL.String(),
102+
Username:ua.FriendlyName,
103+
})
104+
}
74105
}
75106

76107
// ProcessAuthorize handles POST /oauth2/authorize requests to process the user's authorization decision
77-
// and generate an authorization code. GET requests are handled by authorizeMW.
78-
funcProcessAuthorize(db database.Store,accessURL*url.URL) http.HandlerFunc {
79-
handler:=func(rw http.ResponseWriter,r*http.Request) {
108+
// and generate an authorization code.
109+
funcProcessAuthorize(db database.Store) http.HandlerFunc {
110+
returnfunc(rw http.ResponseWriter,r*http.Request) {
80111
ctx:=r.Context()
81112
apiKey:=httpmw.APIKey(r)
82113
app:=httpmw.OAuth2ProviderApp(r)
@@ -159,9 +190,8 @@ func ProcessAuthorize(db database.Store, accessURL *url.URL) http.HandlerFunc {
159190
}
160191
params.redirectURL.RawQuery=newQuery.Encode()
161192

162-
http.Redirect(rw,r,params.redirectURL.String(),http.StatusTemporaryRedirect)
193+
// (ThomasK33): Use a 302 redirect as some (external) OAuth 2 apps and browsers
194+
// do not work with the 307.
195+
http.Redirect(rw,r,params.redirectURL.String(),http.StatusFound)
163196
}
164-
165-
// Always wrap with its custom mw.
166-
returnauthorizeMW(accessURL)(http.HandlerFunc(handler)).ServeHTTP
167197
}

‎coderd/oauth2provider/middleware.go

Lines changed: 0 additions & 83 deletions
This file was deleted.

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp