From 7515102ec6d5293cab85f746c49f34dd03042004 Mon Sep 17 00:00:00 2001 From: Keith James Date: Fri, 25 Oct 2024 13:15:09 +0100 Subject: [PATCH 1/2] Fix hard-coded OIDC callback URL --- docker-compose.yml | 2 ++ server/server.go | 35 ++++++++++++++++++++++++++++------- 2 files changed, 30 insertions(+), 7 deletions(-) diff --git a/docker-compose.yml b/docker-compose.yml index fe40cf3..af1498d 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -26,6 +26,7 @@ services: "--key-file", "/app/config/localhost.key", "--irods-env", "/app/config/app_irods_environment.json", "--enable-oidc", + "--index-interval", "1h", "--log-level", "trace"] # Set the following environment variables in a .env file (files named .env # are declared in .gitignore): @@ -39,6 +40,7 @@ services: # OIDC_CLIENT_ID # OIDC_CLIENT_SECRET # OIDC_ISSUER_URL + # OIDC_CALLBACK_URL env_file: .env ports: - "3333:3333" diff --git a/server/server.go b/server/server.go index 019ef36..d51d2b5 100644 --- a/server/server.go +++ b/server/server.go @@ -26,6 +26,7 @@ import ( "net" "net/http" "net/mail" + "net/url" "os" "os/signal" "path/filepath" @@ -82,9 +83,10 @@ const ( ) const ( - EnvClientID = "OIDC_CLIENT_ID" - EnvClientSecret = "OIDC_CLIENT_SECRET" - EnvOIDCIssuer = "OIDC_ISSUER_URL" + EnvClientID = "OIDC_CLIENT_ID" + EnvClientSecret = "OIDC_CLIENT_SECRET" + EnvOIDCIssuerURL = "OIDC_ISSUER_URL" + EnvOIDCRedirectURL = "OIDC_CALLBACK_URL" ) const ( @@ -162,7 +164,8 @@ func NewSqyrrlServer(logger zerolog.Logger, config Config) (server *SqyrrlServer var oidcConfig *oidc.Config var oidcProvider *oidc.Provider var oauth2Config *oauth2.Config - var clientID, clientSecret, oidcIssuer string + var clientID, clientSecret, oidcIssuerURL, oidcRedirectURL string + var issuerURL, redirectURL *url.URL if config.EnableOIDC { if clientID, err = getEnv(EnvClientID); err != nil { @@ -171,7 +174,10 @@ func NewSqyrrlServer(logger zerolog.Logger, config Config) (server *SqyrrlServer if clientSecret, err = getEnv(EnvClientSecret); err != nil { return nil, err } - if oidcIssuer, err = getEnv(EnvOIDCIssuer); err != nil { + if oidcIssuerURL, err = getEnv(EnvOIDCIssuerURL); err != nil { + return nil, err + } + if oidcRedirectURL, err = getEnv(EnvOIDCRedirectURL); err != nil { return nil, err } @@ -179,7 +185,22 @@ func NewSqyrrlServer(logger zerolog.Logger, config Config) (server *SqyrrlServer ClientID: clientID, } - oidcProvider, err = oidc.NewProvider(context.Background(), oidcIssuer) + // Parse the provided URLs to ensure they are valid + issuerURL, err = url.Parse(oidcIssuerURL) + if err != nil { + return nil, err + } + redirectURL, err = url.Parse(oidcRedirectURL) + if err != nil { + return nil, err + } + redirectURL, err = url.Parse(redirectURL.Scheme + "://" + + net.JoinHostPort(redirectURL.Hostname(), config.Port)) + if err != nil { + return nil, err + } + + oidcProvider, err = oidc.NewProvider(context.Background(), issuerURL.String()) if err != nil { return nil, err } @@ -188,7 +209,7 @@ func NewSqyrrlServer(logger zerolog.Logger, config Config) (server *SqyrrlServer ClientID: clientID, ClientSecret: clientSecret, Endpoint: oidcProvider.Endpoint(), - RedirectURL: "https://" + net.JoinHostPort("localhost", config.Port) + EndpointAuthCallback, + RedirectURL: redirectURL.JoinPath(EndpointAuthCallback).String(), Scopes: []string{oidc.ScopeOpenID, "profile", "email"}, } From 57bf71e8da03ab5ea90b0cca1a8856a1ec650ac9 Mon Sep 17 00:00:00 2001 From: Keith James Date: Fri, 25 Oct 2024 14:35:21 +0100 Subject: [PATCH 2/2] Fix hard-coded "localhost" OIDC redirect URL Fix login endpoint accepting GET where it should accept POST Add additional parsing/checking of URLs supplied in configurations Update from Go 1.22 to 1.23 --- Dockerfile | 2 +- docker-compose.yml | 2 +- go.mod | 2 +- server/handlers.go | 4 ++++ server/routes.go | 9 +++++---- 5 files changed, 12 insertions(+), 7 deletions(-) diff --git a/Dockerfile b/Dockerfile index f881a05..65c38f0 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,5 +1,5 @@ -FROM golang:1.22 AS builder +FROM golang:1.23 AS builder WORKDIR /app diff --git a/docker-compose.yml b/docker-compose.yml index af1498d..59fa9a8 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -26,7 +26,7 @@ services: "--key-file", "/app/config/localhost.key", "--irods-env", "/app/config/app_irods_environment.json", "--enable-oidc", - "--index-interval", "1h", + "--index-interval", "60s", "--log-level", "trace"] # Set the following environment variables in a .env file (files named .env # are declared in .gitignore): diff --git a/go.mod b/go.mod index e0fe6c4..5aa9ed1 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module sqyrrl -go 1.22 +go 1.23 require ( github.com/alexedwards/scs/v2 v2.8.0 diff --git a/server/handlers.go b/server/handlers.go index 31f1725..21bef11 100644 --- a/server/handlers.go +++ b/server/handlers.go @@ -50,7 +50,9 @@ func HandleHomePage(server *SqyrrlServer) http.Handler { requestPath := r.URL.Path requestMethod := r.Method + // Redirect all GET requests to the iRODS API if requestPath != "/" && requestMethod == "GET" { + // No favicon.ico here. Prevent this redirecting to look in iRODS for it if requestPath == "/favicon.ico" { writeErrorResponse(logger, w, http.StatusNotFound) return @@ -214,6 +216,8 @@ func HandleAuthCallback(server *SqyrrlServer) http.Handler { Str("email", claims.Email). Msg("User logged in") + logger.Debug().Msg("Redirecting logged in user to home page") + http.Redirect(w, r, "/", http.StatusFound) }) } diff --git a/server/routes.go b/server/routes.go index 201272a..b90afad 100644 --- a/server/routes.go +++ b/server/routes.go @@ -47,12 +47,13 @@ func (server *SqyrrlServer) addRoutes(mux *http.ServeMux) { getStatic := http.StripPrefix(EndpointStatic, HandleStaticContent(server)) getObject := http.StripPrefix(EndpointIRODS, HandleIRODSGet(server)) + // See the home page template for the login/logout button that POSTs to these endpoints loginHandler := sm.LoadAndSave(correlate(logRequest(HandleLogin(server)))) - server.addRoute(mux, "GET", EndpointLogin, loginHandler) - + server.addRoute(mux, "POST", EndpointLogin, loginHandler) logoutHandler := sm.LoadAndSave(correlate(logRequest(HandleLogout(server)))) server.addRoute(mux, "POST", EndpointLogout, logoutHandler) + // OIDC authentication callback endpoint authCallbackHandler := sm.LoadAndSave(correlate(logRequest(HandleAuthCallback(server)))) server.addRoute(mux, "GET", EndpointAuthCallback, authCallbackHandler) @@ -61,12 +62,12 @@ func (server *SqyrrlServer) addRoutes(mux *http.ServeMux) { staticHandler := sm.LoadAndSave(sanitiseURL(correlate(logRequest(getStatic)))) server.addRoute(mux, "GET", EndpointStatic, staticHandler) - // The endpoint used to access files in iRODS + // The API endpoint used to access files in iRODS irodsGetHandler := sm.LoadAndSave(sanitiseURL(correlate(logRequest(getObject)))) server.addRoute(mux, "GET", EndpointIRODS, irodsGetHandler) // The root endpoint hosts a home page. Any requests relative to it are redirected - // to the API endpoint + // to the iRODS API endpoint rootHandler := sm.LoadAndSave(sanitiseURL(correlate(logRequest(HandleHomePage(server))))) server.addRoute(mux, "GET", EndpointRoot, rootHandler) }