Skip to content

Commit 6f159c6

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 6f159c6

File tree

3 files changed

+43
-96
lines changed

3 files changed

+43
-96
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: 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
type authorizeParams struct {
@@ -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-
func ShowAuthorizePage(db database.Store, accessURL *url.URL) http.HandlerFunc {
72-
handler := authorizeMW(accessURL)(ProcessAuthorize(db, accessURL))
73-
return handler.ServeHTTP
71+
func ShowAuthorizePage(accessURL *url.URL) http.HandlerFunc {
72+
return func(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+
if err != 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+
if err != nil {
84+
errStr := make([]string, len(validationErrs))
85+
for i, err := range validationErrs {
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-
func ProcessAuthorize(db database.Store, accessURL *url.URL) http.HandlerFunc {
79-
handler := func(rw http.ResponseWriter, r *http.Request) {
108+
// and generate an authorization code.
109+
func ProcessAuthorize(db database.Store) http.HandlerFunc {
110+
return func(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-
return authorizeMW(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)