Skip to content

Commit ee2ff90

Browse files
committed
refactor(oauth2): simplify authorization flow by removing middleware layer
Change-Id: Ieff16b08aeb2cf2357ada11d83fd408cc66c6c5a Signed-off-by: Thomas Kosiewski <tk@coder.com>
1 parent 071383b commit ee2ff90

File tree

3 files changed

+42
-94
lines changed

3 files changed

+42
-94
lines changed

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-
return oauth2provider.ShowAuthorizePage(api.Database, api.AccessURL)
119+
return oauth2provider.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-
return oauth2provider.ProcessAuthorize(api.Database, api.AccessURL)
134+
return oauth2provider.ProcessAuthorize(api.Database)
135135
}
136136

137137
// @Summary OAuth2 token exchange.

coderd/oauth2provider/authorize.go

Lines changed: 40 additions & 9 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
type authorizeParams struct {
@@ -68,15 +69,46 @@ func extractAuthorizeParams(r *http.Request, callbackURL *url.URL) (authorizePar
6869

6970
// ShowAuthorizePage handles GET /oauth2/authorize requests to display the HTML authorization page.
7071
// It uses authorizeMW which intercepts GET requests to show the authorization form.
71-
func ShowAuthorizePage(db database.Store, accessURL *url.URL) http.HandlerFunc {
72-
handler := authorizeMW(accessURL)(ProcessAuthorize(db, accessURL))
73-
return handler.ServeHTTP
72+
func ShowAuthorizePage(accessURL *url.URL) http.HandlerFunc {
73+
return func(rw http.ResponseWriter, r *http.Request) {
74+
app := httpmw.OAuth2ProviderApp(r)
75+
ua := httpmw.UserAuthorization(r.Context())
76+
77+
callbackURL, err := url.Parse(app.CallbackURL)
78+
if err != nil {
79+
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})
80+
return
81+
}
82+
83+
params, validationErrs, err := extractAuthorizeParams(r, callbackURL)
84+
if err != nil {
85+
errStr := make([]string, len(validationErrs))
86+
for i, err := range validationErrs {
87+
errStr[i] = err.Detail
88+
}
89+
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})
90+
return
91+
}
92+
93+
cancel := params.redirectURL
94+
cancelQuery := params.redirectURL.Query()
95+
cancelQuery.Add("error", "access_denied")
96+
cancel.RawQuery = cancelQuery.Encode()
97+
98+
site.RenderOAuthAllowPage(rw, r, site.RenderOAuthAllowData{
99+
AppIcon: app.Icon,
100+
AppName: app.Name,
101+
CancelURI: cancel.String(),
102+
RedirectURI: r.URL.String(),
103+
Username: ua.FriendlyName,
104+
})
105+
}
74106
}
75107

76108
// ProcessAuthorize handles POST /oauth2/authorize requests to process the user's authorization decision
77109
// and generate an authorization code. GET requests are handled by authorizeMW.
78-
func ProcessAuthorize(db database.Store, accessURL *url.URL) http.HandlerFunc {
79-
handler := func(rw http.ResponseWriter, r *http.Request) {
110+
func ProcessAuthorize(db database.Store) http.HandlerFunc {
111+
return func(rw http.ResponseWriter, r *http.Request) {
80112
ctx := r.Context()
81113
apiKey := httpmw.APIKey(r)
82114
app := httpmw.OAuth2ProviderApp(r)
@@ -159,9 +191,8 @@ func ProcessAuthorize(db database.Store, accessURL *url.URL) http.HandlerFunc {
159191
}
160192
params.redirectURL.RawQuery = newQuery.Encode()
161193

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

coderd/oauth2provider/middleware.go

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

0 commit comments

Comments
 (0)