refactor(oauth): improve readability of newOAuthHandler and OAuth methods.

This commit is contained in:
Miroslav Šedivý
2026-09-24 00:17:34 +02:00
parent 11b6c8d8f6
commit d991fec3c7
4 changed files with 40 additions and 4 deletions
+32 -2
View File
@@ -32,7 +32,13 @@ func newOAuthHandler(service *oauth.Service, pathPrefix string, trustProxy, prov
if pathPrefix == "" { if pathPrefix == "" {
pathPrefix = "/" pathPrefix = "/"
} }
return &oauthHandler{service: service, pathPrefix: pathPrefix, trustProxy: trustProxy, providerOAuth: providerOAuth}
return &oauthHandler{
service: service,
pathPrefix: pathPrefix,
trustProxy: trustProxy,
providerOAuth: providerOAuth,
}
} }
func (api *ApiManagerCtx) OAuthConfig(w http.ResponseWriter, r *http.Request) error { func (api *ApiManagerCtx) OAuthConfig(w http.ResponseWriter, r *http.Request) error {
@@ -48,17 +54,21 @@ func (api *ApiManagerCtx) OAuthLogin(w http.ResponseWriter, r *http.Request) err
if !api.oauth.providerOAuth || !api.oauth.service.Enabled() { if !api.oauth.providerOAuth || !api.oauth.service.Enabled() {
return utils.HttpNotFound() return utils.HttpNotFound()
} }
if !api.sessions.CookieEnabled() { if !api.sessions.CookieEnabled() {
return utils.HttpError(http.StatusServiceUnavailable, "OAuth requires session cookies to be enabled") return utils.HttpError(http.StatusServiceUnavailable, "OAuth requires session cookies to be enabled")
} }
callbackURL, err := api.oauth.callbackURL(r) callbackURL, err := api.oauth.callbackURL(r)
if err != nil { if err != nil {
return utils.HttpInternalServerError("unable to determine OAuth callback URL").WithInternalErr(err) return utils.HttpInternalServerError("unable to determine OAuth callback URL").WithInternalErr(err)
} }
location, err := api.oauth.service.Start(r.Context(), callbackURL) location, err := api.oauth.service.Start(r.Context(), callbackURL)
if err != nil { if err != nil {
return oauthServiceError(err, true) return oauthServiceError(err, true)
} }
http.Redirect(w, r, location, http.StatusFound) http.Redirect(w, r, location, http.StatusFound)
return nil return nil
} }
@@ -67,14 +77,17 @@ func (api *ApiManagerCtx) OAuthCallback(w http.ResponseWriter, r *http.Request)
if !api.oauth.providerOAuth || !api.oauth.service.Enabled() { if !api.oauth.providerOAuth || !api.oauth.service.Enabled() {
return utils.HttpNotFound() return utils.HttpNotFound()
} }
if providerError := r.URL.Query().Get("error"); providerError != "" { if providerError := r.URL.Query().Get("error"); providerError != "" {
return utils.HttpUnauthorized("OAuth authorization was declined").WithInternalMsg(providerError) return utils.HttpUnauthorized("OAuth authorization was declined").WithInternalMsg(providerError)
} }
_, token, err := api.oauth.service.Complete(r.Context(), r.URL.Query().Get("state"), r.URL.Query().Get("code")) _, token, err := api.oauth.service.Complete(r.Context(), r.URL.Query().Get("state"), r.URL.Query().Get("code"))
if err != nil { if err != nil {
return oauthServiceError(err, false) return oauthServiceError(err, false)
} }
api.sessions.CookieSetToken(w, token) api.sessions.CookieSetToken(w, token)
http.Redirect(w, r, api.oauth.successRedirect(), http.StatusSeeOther) http.Redirect(w, r, api.oauth.successRedirect(), http.StatusSeeOther)
return nil return nil
} }
@@ -83,21 +96,27 @@ func oauthServiceError(err error, start bool) error {
if errors.Is(err, types.ErrSessionAlreadyConnected) { if errors.Is(err, types.ErrSessionAlreadyConnected) {
return utils.HttpUnprocessableEntity("session already connected") return utils.HttpUnprocessableEntity("session already connected")
} }
if errors.Is(err, types.ErrSessionLoginsLocked) { if errors.Is(err, types.ErrSessionLoginsLocked) {
return utils.HttpForbidden("logins are locked").WithInternalErr(err) return utils.HttpForbidden("logins are locked").WithInternalErr(err)
} }
if start && strings.HasPrefix(err.Error(), "issuer discovery:") { if start && strings.HasPrefix(err.Error(), "issuer discovery:") {
return utils.HttpError(http.StatusServiceUnavailable, "OAuth issuer discovery failed").WithInternalErr(err) return utils.HttpError(http.StatusServiceUnavailable, "OAuth issuer discovery failed").WithInternalErr(err)
} }
if start { if start {
return utils.HttpError(http.StatusServiceUnavailable, err.Error()).WithInternalErr(err) return utils.HttpError(http.StatusServiceUnavailable, err.Error()).WithInternalErr(err)
} }
if strings.Contains(err.Error(), "missing state or code") || strings.Contains(err.Error(), "state is invalid") { if strings.Contains(err.Error(), "missing state or code") || strings.Contains(err.Error(), "state is invalid") {
return utils.HttpBadRequest(err.Error()).WithInternalErr(err) return utils.HttpBadRequest(err.Error()).WithInternalErr(err)
} }
if strings.HasPrefix(err.Error(), "issuer discovery:") || strings.Contains(err.Error(), "not fully configured") { if strings.HasPrefix(err.Error(), "issuer discovery:") || strings.Contains(err.Error(), "not fully configured") {
return utils.HttpError(http.StatusServiceUnavailable, "OAuth issuer discovery failed").WithInternalErr(err) return utils.HttpError(http.StatusServiceUnavailable, "OAuth issuer discovery failed").WithInternalErr(err)
} }
return utils.HttpUnauthorized("OAuth authorization failed").WithInternalErr(err) return utils.HttpUnauthorized("OAuth authorization failed").WithInternalErr(err)
} }
@@ -106,6 +125,7 @@ func (handler *oauthHandler) callbackURL(r *http.Request) (string, error) {
if r.TLS != nil { if r.TLS != nil {
scheme = "https" scheme = "https"
} }
host := r.Host host := r.Host
if handler.trustProxy { if handler.trustProxy {
if value := forwardedHeaderValue(r.Header.Get("X-Forwarded-Proto")); value != "" { if value := forwardedHeaderValue(r.Header.Get("X-Forwarded-Proto")); value != "" {
@@ -115,22 +135,32 @@ func (handler *oauthHandler) callbackURL(r *http.Request) (string, error) {
host = value host = value
} }
} }
if host == "" || (scheme != "http" && scheme != "https") { if host == "" || (scheme != "http" && scheme != "https") {
return "", errors.New("request has no valid public URL") return "", errors.New("request has no valid public URL")
} }
callback := (&url.URL{Scheme: scheme, Host: host, Path: path.Join(handler.pathPrefix, "/api/oauth/callback")}).String()
callback := (&url.URL{
Scheme: scheme,
Host: host,
Path: path.Join(handler.pathPrefix, "/api/oauth/callback"),
}).String()
if parsed, err := url.Parse(callback); err != nil || parsed.Host == "" { if parsed, err := url.Parse(callback); err != nil || parsed.Host == "" {
return "", errors.New("request has no valid public URL") return "", errors.New("request has no valid public URL")
} }
return callback, nil return callback, nil
} }
func (handler *oauthHandler) successRedirect() string { func (handler *oauthHandler) successRedirect() string {
redirect := handler.service.SuccessRedirect() redirect := handler.service.SuccessRedirect()
if strings.HasPrefix(redirect, "/") && !strings.HasPrefix(redirect, "//") { if strings.HasPrefix(redirect, "/") && !strings.HasPrefix(redirect, "//") {
return path.Join(handler.pathPrefix, redirect) return path.Join(handler.pathPrefix, redirect)
} }
return handler.pathPrefix return handler.pathPrefix
} }
func forwardedHeaderValue(value string) string { func forwardedHeaderValue(value string) string {
return strings.TrimSpace(strings.Split(value, ",")[0]) return strings.TrimSpace(strings.Split(value, ",")[0])
} }
+6 -1
View File
@@ -43,7 +43,12 @@ func New(
desktop: desktop, desktop: desktop,
capture: capture, capture: capture,
routers: make(map[string]func(types.Router)), routers: make(map[string]func(types.Router)),
oauth: newOAuthHandler(oauth.NewService(memberConfig.OAuth, sessions), pathPrefix, serverConfig != nil && serverConfig.Proxy, memberConfig.Provider == "oauth"), oauth: newOAuthHandler(
oauth.NewService(memberConfig.OAuth, sessions),
pathPrefix,
serverConfig != nil && serverConfig.Proxy,
memberConfig.Provider == "oauth",
),
} }
} }
+1
View File
@@ -67,6 +67,7 @@ func (api *ApiManagerCtx) Logout(w http.ResponseWriter, r *http.Request) error {
return utils.HttpInternalServerError().WithInternalErr(err) return utils.HttpInternalServerError().WithInternalErr(err)
} }
} }
if api.sessions.CookieEnabled() { if api.sessions.CookieEnabled() {
api.sessions.CookieClearToken(w, r) api.sessions.CookieClearToken(w, r)
} }
+1 -1
View File
@@ -25,7 +25,7 @@ A member profile is a structure that describes the user and what the user is all
| Field | Description | Type | | Field | Description | Type |
|----------------------------|-------------|------| |----------------------------|-------------|------|
| <Def id="profile.name" /> | User's name as shown in the UI, must not be unique within the system (not used as an identifier). | string | | <Def id="profile.name" /> | User's name as shown in the UI, must not be unique within the system (not used as an identifier). | string |
| `avatar` | Avatar URL shown in clients that support member avatars. OAuth logins synchronize this value from the configured user-info field. | string | | <Def id="profile.avatar" /> | Avatar URL shown in clients that support member avatars. OAuth logins synchronize this value from the configured user-info field. | string |
| <Def id="profile.is_admin" /> | Whether the user can perform administrative tasks that include managing users, sessions, and settings. | boolean | | <Def id="profile.is_admin" /> | Whether the user can perform administrative tasks that include managing users, sessions, and settings. | boolean |
| <Def id="profile.can_login" /> | Whether the user can log in to the system and use the HTTP API. | boolean | | <Def id="profile.can_login" /> | Whether the user can log in to the system and use the HTTP API. | boolean |
| <Def id="profile.can_connect" /> | Whether the user can connect to the room using the WebSocket API (needs <Opt id="profile.can_login" /> to be enabled). | boolean | | <Def id="profile.can_connect" /> | Whether the user can connect to the room using the WebSocket API (needs <Opt id="profile.can_login" /> to be enabled). | boolean |