diff --git a/apps/standalone/src/app/components/Login/LoginPage.tsx b/apps/standalone/src/app/components/Login/LoginPage.tsx index 58c611fca..f93ce2352 100644 --- a/apps/standalone/src/app/components/Login/LoginPage.tsx +++ b/apps/standalone/src/app/components/Login/LoginPage.tsx @@ -9,7 +9,7 @@ import { useFetch } from '@flightctl/ui-components/src/hooks/useFetch'; import { useTranslation } from '@flightctl/ui-components/src/hooks/useTranslation'; import { getProviderDisplayName } from '@flightctl/ui-components/src/utils/authProvider'; -import { apiProxy } from '../../utils/apiCalls'; +import { JUST_LOGGED_OUT_KEY, apiProxy } from '../../utils/apiCalls'; import LoginPageLayout from './LoginPageLayout'; const redirectToProviderLogin = async (provider: AuthProvider) => { @@ -38,6 +38,7 @@ const LoginPage = () => { const [userSelectedProvider, setUserSelectedProvider] = React.useState(null); const [defaultProviderName, setDefaultProviderName] = React.useState(''); const [isRedirecting, setIsRedirecting] = React.useState(false); + const [suppressAutoSelect, setSuppressAutoSelect] = React.useState(false); const handleProviderSelect = async (provider: AuthProvider) => { // Prevent multiple clicks while redirect is in progress @@ -83,7 +84,12 @@ const LoginPage = () => { if (providers.length > 0) { setProviders(providers); setDefaultProviderName(config.defaultProvider || ''); - if (providers.length === 1 && providers[0].spec.providerType !== ProviderType.K8s) { + const justLoggedOut = sessionStorage.getItem(JUST_LOGGED_OUT_KEY) === 'true'; + if (justLoggedOut) { + sessionStorage.removeItem(JUST_LOGGED_OUT_KEY); + setSuppressAutoSelect(true); + } + if (!justLoggedOut && providers.length === 1 && providers[0].spec.providerType !== ProviderType.K8s) { setIsRedirecting(true); try { await redirectToProviderLogin(providers[0]); @@ -125,7 +131,8 @@ const LoginPage = () => { ); } - const selectedProvider = userSelectedProvider || (providers.length === 1 ? providers[0] : null); + const selectedProvider = + userSelectedProvider || (suppressAutoSelect ? null : providers.length === 1 ? providers[0] : null); if (selectedProvider?.spec.providerType === ProviderType.K8s) { return ( { return { api: apiName, url: `${apiProxy}/flightctl/api/v1/${path}` }; }; +export const JUST_LOGGED_OUT_KEY = 'flightctl.justLoggedOut'; + export const logout = async () => { const redirectBase = encodeURIComponent(window.location.origin); const response = await fetch(`${apiProxy}/logout?redirect_base=${redirectBase}`, { @@ -74,7 +76,8 @@ export const logout = async () => { if (url) { window.location.href = url; } else { - window.location.reload(); + sessionStorage.setItem(JUST_LOGGED_OUT_KEY, 'true'); + window.location.href = '/login'; } }; diff --git a/package-lock.json b/package-lock.json index 8b04e13a8..f01d15dff 100644 --- a/package-lock.json +++ b/package-lock.json @@ -248,24 +248,6 @@ "js-yaml": "bin/js-yaml.js" } }, - "libs/ansible": { - "name": "@flightctl/ansible", - "version": "0.0.0", - "extraneous": true, - "license": "MIT", - "dependencies": { - "react-router-dom": "^6.22.0" - } - }, - "libs/cypress": { - "name": "@flightctl/ui-tests-cypress", - "version": "0.0.0", - "extraneous": true, - "license": "MIT", - "devDependencies": { - "cypress": "^14.0.0" - } - }, "libs/i18n": { "name": "@flightctl/i18n", "version": "0.0.0", @@ -4139,101 +4121,6 @@ "dev": true, "license": "MIT" }, - "node_modules/@swc/core": { - "version": "1.3.104", - "resolved": "https://registry.npmjs.org/@swc/core/-/core-1.3.104.tgz", - "integrity": "sha512-9LWH/qzR/Pmyco+XwPiPfz59T1sryI7o5dmqb593MfCkaX5Fzl9KhwQTI47i21/bXYuCdfa9ySZuVkzXMirYxA==", - "dev": true, - "hasInstallScript": true, - "license": "Apache-2.0", - "optional": true, - "peer": true, - "dependencies": { - "@swc/counter": "^0.1.1", - "@swc/types": "^0.1.5" - }, - "engines": { - "node": ">=10" - }, - "funding": { - "type": "opencollective", - "url": "https://opencollective.com/swc" - }, - "optionalDependencies": { - "@swc/core-darwin-arm64": "1.3.104", - "@swc/core-darwin-x64": "1.3.104", - "@swc/core-linux-arm-gnueabihf": "1.3.104", - "@swc/core-linux-arm64-gnu": "1.3.104", - "@swc/core-linux-arm64-musl": "1.3.104", - "@swc/core-linux-x64-gnu": "1.3.104", - "@swc/core-linux-x64-musl": "1.3.104", - "@swc/core-win32-arm64-msvc": "1.3.104", - "@swc/core-win32-ia32-msvc": "1.3.104", - "@swc/core-win32-x64-msvc": "1.3.104" - }, - "peerDependencies": { - "@swc/helpers": "^0.5.0" - }, - "peerDependenciesMeta": { - "@swc/helpers": { - "optional": true - } - } - }, - "node_modules/@swc/core-linux-x64-gnu": { - "version": "1.3.104", - "resolved": "https://registry.npmjs.org/@swc/core-linux-x64-gnu/-/core-linux-x64-gnu-1.3.104.tgz", - "integrity": "sha512-MfX/wiRdTjE5uXHTDnaX69xI4UBfxIhcxbVlMj//N+7AX/G2pl2UFityfVMU2HpM12BRckrCxVI8F/Zy3DZkYQ==", - "cpu": [ - "x64" - ], - "dev": true, - "license": "Apache-2.0 AND MIT", - "optional": true, - "os": [ - "linux" - ], - "peer": true, - "engines": { - "node": ">=10" - } - }, - "node_modules/@swc/core-linux-x64-musl": { - "version": "1.3.104", - "resolved": "https://registry.npmjs.org/@swc/core-linux-x64-musl/-/core-linux-x64-musl-1.3.104.tgz", - "integrity": "sha512-5yeILaxA31gGEmquErO8yxlq1xu0XVt+fz5mbbKXKZMRRILxYxNzAGb5mzV41r0oHz6Vhv4AXX/WMCmeWl+HkQ==", - "cpu": [ - "x64" - ], - "dev": true, - "license": "Apache-2.0 AND MIT", - "optional": true, - "os": [ - "linux" - ], - "peer": true, - "engines": { - "node": ">=10" - } - }, - "node_modules/@swc/counter": { - "version": "0.1.2", - "resolved": "https://registry.npmjs.org/@swc/counter/-/counter-0.1.2.tgz", - "integrity": "sha512-9F4ys4C74eSTEUNndnER3VJ15oru2NumfQxS8geE+f3eB5xvfxpWyqE5XlVnxb/R14uoXi6SLbBwwiDSkv+XEw==", - "dev": true, - "license": "Apache-2.0", - "optional": true, - "peer": true - }, - "node_modules/@swc/types": { - "version": "0.1.5", - "resolved": "https://registry.npmjs.org/@swc/types/-/types-0.1.5.tgz", - "integrity": "sha512-myfUej5naTBWnqOCc/MdVOLVjXUXtIA+NpDrDBKJtLLg2shUjBu3cZmB/85RyitKc55+lUUyl7oRfLOvkr2hsw==", - "dev": true, - "license": "Apache-2.0", - "optional": true, - "peer": true - }, "node_modules/@tsconfig/node10": { "version": "1.0.9", "resolved": "https://registry.npmjs.org/@tsconfig/node10/-/node10-1.0.9.tgz", @@ -8887,31 +8774,6 @@ "node": ">= 0.8" } }, - "node_modules/encoding": { - "version": "0.1.13", - "resolved": "https://registry.npmjs.org/encoding/-/encoding-0.1.13.tgz", - "integrity": "sha512-ETBauow1T35Y/WZMkio9jiM0Z5xjHHmJ4XmjZOq1l/dXz3lr2sRn87nJy20RupqSh1F2m3HHPSp8ShIPQJrJ3A==", - "license": "MIT", - "optional": true, - "peer": true, - "dependencies": { - "iconv-lite": "^0.6.2" - } - }, - "node_modules/encoding/node_modules/iconv-lite": { - "version": "0.6.3", - "resolved": "https://registry.npmjs.org/iconv-lite/-/iconv-lite-0.6.3.tgz", - "integrity": "sha512-4fCk79wshMdzMp2rH06qWrJE4iolqLhCUH+OiuIgU++RB0+94NlDL81atO7GX55uUKueo0txHNtvEyI6D7WdMw==", - "license": "MIT", - "optional": true, - "peer": true, - "dependencies": { - "safer-buffer": ">= 2.1.2 < 3.0.0" - }, - "engines": { - "node": ">=0.10.0" - } - }, "node_modules/end-of-stream": { "version": "1.4.4", "resolved": "https://registry.npmjs.org/end-of-stream/-/end-of-stream-1.4.4.tgz", diff --git a/proxy/auth/helpers_test.go b/proxy/auth/helpers_test.go new file mode 100644 index 000000000..3ee6c6b64 --- /dev/null +++ b/proxy/auth/helpers_test.go @@ -0,0 +1,30 @@ +package auth + +import ( + "fmt" + "net/http" + "net/url" + "os" + "testing" + + "github.com/flightctl/flightctl-ui/log" +) + +func TestMain(m *testing.M) { + log.InitLogs() + os.Exit(m.Run()) +} + +// redirectBaseMatchesRequest is a test helper that checks whether url u's origin +// matches the effective request origin. Returns nil when they match, error otherwise. +func redirectBaseMatchesRequest(t *testing.T, r *http.Request, u *url.URL) error { + t.Helper() + ok, err := isSameSchemeAndHost(u.String(), r) + if err != nil { + return err + } + if !ok { + return fmt.Errorf("origin mismatch: redirect base %s://%s does not match request origin", u.Scheme, u.Host) + } + return nil +} diff --git a/proxy/auth/openshift.go b/proxy/auth/openshift.go index 07ccee50a..195315952 100644 --- a/proxy/auth/openshift.go +++ b/proxy/auth/openshift.go @@ -1,13 +1,17 @@ package auth import ( + "crypto/sha256" "crypto/tls" + b64 "encoding/base64" "fmt" "net/http" "net/url" "strings" + "time" "github.com/flightctl/flightctl-ui/bridge" + "github.com/flightctl/flightctl-ui/log" "github.com/flightctl/flightctl/api/v1beta1" "github.com/openshift/osincli" ) @@ -47,6 +51,9 @@ func getOpenShiftAuthHandlerFromSpec(provider *v1beta1.AuthProvider, openshiftSp } apiServerURL = parsedURL.String() } else { + // url.Parse should not fail on a URL already validated by the API + // server; fall back to the raw string to preserve backward + // compatibility rather than failing provider initialization. apiServerURL = authURL } } else { @@ -101,6 +108,8 @@ func getOpenShiftAuthHandlerFromSpec(provider *v1beta1.AuthProvider, openshiftSp return handler, nil } +// openshiftClientForRedirect creates an osincli OAuth2 client configured for +// the given redirect URI using the handler's stored credentials and TLS config. func (o *OpenShiftAuthHandler) openshiftClientForRedirect(redirectURI string) (*osincli.Client, error) { oauthClientConfig := &osincli.ClientConfig{ ClientId: o.clientId, @@ -123,11 +132,76 @@ func (o *OpenShiftAuthHandler) openshiftClientForRedirect(redirectURI string) (* return client, nil } +// openShiftTokenName derives the OAuthAccessToken Kubernetes resource name from +// an access token value. Tokens prefixed with "sha256~" are named by hashing +// the full token string with SHA-256 and base64url-encoding the result (no +// padding). Legacy tokens (no prefix) are used as-is — the token value IS the +// resource name in that case. +func openShiftTokenName(token string) string { + if strings.HasPrefix(token, "sha256~") { + hash := sha256.Sum256([]byte(token)) + return "sha256~" + b64.RawURLEncoding.EncodeToString(hash[:]) + } + return token +} + +// openShiftAPIServerBase normalises apiServerURL to a plain scheme+host (no +// path, query, or fragment) so the revocation URL is always well-formed even +// when apiServerURL was derived from an AuthorizationUrl that carries extra +// components. +func openShiftAPIServerBase(rawURL string) string { + parsed, err := url.Parse(rawURL) + if err != nil || parsed.Host == "" { + return strings.TrimSuffix(rawURL, "/") + } + return parsed.Scheme + "://" + parsed.Host +} + +// Logout revokes the OpenShift OAuth access token server-side by issuing a +// DELETE to /apis/oauth.openshift.io/v1/oauthaccesstokens/{name}, authenticated +// with the token itself as the Bearer credential. Revocation invalidates the +// OpenShift session without requiring a browser redirect, avoiding the 405 that +// the OAuth server's /logout endpoint returns for unauthenticated GET requests. +// +// Always returns ("", nil) — the caller falls back to a local page reload which +// lands the user on the login page with no valid session. func (o *OpenShiftAuthHandler) Logout(token string, _ string) (string, error) { - // The cookie will be cleared by the proxy + if token == "" || o.apiServerURL == "" { + return "", nil + } + + tokenName := openShiftTokenName(token) + base := openShiftAPIServerBase(o.apiServerURL) + revokeURL := fmt.Sprintf("%s/apis/oauth.openshift.io/v1/oauthaccesstokens/%s", base, tokenName) + + client := &http.Client{ + Transport: &http.Transport{TLSClientConfig: o.tlsConfig}, + Timeout: 10 * time.Second, + } + + req, err := http.NewRequest(http.MethodDelete, revokeURL, nil) + if err != nil { + log.GetLogger().WithError(err).Warn("Failed to build OpenShift token revocation request") + return "", nil + } + req.Header.Set("Authorization", "Bearer "+token) + + resp, err := client.Do(req) + if err != nil { + log.GetLogger().WithError(err).Warn("Failed to revoke OpenShift OAuth token") + return "", nil + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusNoContent { + log.GetLogger().Warnf("OpenShift token revocation returned unexpected status %d", resp.StatusCode) + } + return "", nil } +// GetLoginRedirectURL returns the OAuth2 authorization URL the browser should +// be redirected to in order to initiate the OpenShift login flow. func (o *OpenShiftAuthHandler) GetLoginRedirectURL(state string, codeChallenge string, redirectURI string) (string, error) { client, err := o.openshiftClientForRedirect(redirectURI) if err != nil { diff --git a/proxy/auth/openshift_test.go b/proxy/auth/openshift_test.go new file mode 100644 index 000000000..de1b79840 --- /dev/null +++ b/proxy/auth/openshift_test.go @@ -0,0 +1,218 @@ +package auth + +import ( + "crypto/sha256" + b64 "encoding/base64" + "net/http" + "net/http/httptest" + "testing" +) + +// TestOpenShiftTokenName verifies the OAuthAccessToken Kubernetes resource name +// derivation: sha256~-prefixed tokens are hashed; legacy tokens are used as-is. +func TestOpenShiftTokenName(t *testing.T) { + t.Parallel() + + sha256Hash := func(s string) string { + hash := sha256.Sum256([]byte(s)) + return "sha256~" + b64.RawURLEncoding.EncodeToString(hash[:]) + } + + tests := []struct { + name string + token string + wantName string + }{ + { + name: "sha256~ prefix token is hashed", + token: "sha256~testtoken", + wantName: sha256Hash("sha256~testtoken"), + }, + { + name: "legacy token used as-is", + token: "opaque-legacy-token", + wantName: "opaque-legacy-token", + }, + { + name: "empty token unchanged", + token: "", + wantName: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got := openShiftTokenName(tt.token) + if got != tt.wantName { + t.Fatalf("expected %q, got %q", tt.wantName, got) + } + }) + } +} + +// TestOpenShiftAPIServerBase verifies that openShiftAPIServerBase normalises +// the base URL by stripping any path, query, and fragment components. +func TestOpenShiftAPIServerBase(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + rawURL string + wantURL string + }{ + { + name: "plain base URL is unchanged", + rawURL: "https://api.cluster.example.com", + wantURL: "https://api.cluster.example.com", + }, + { + name: "trailing slash is removed", + rawURL: "https://api.cluster.example.com/", + wantURL: "https://api.cluster.example.com", + }, + { + name: "path is stripped", + rawURL: "https://api.cluster.example.com/oauth/authorize", + wantURL: "https://api.cluster.example.com", + }, + { + name: "query string is stripped", + rawURL: "https://api.cluster.example.com?foo=bar", + wantURL: "https://api.cluster.example.com", + }, + { + name: "fragment is stripped", + rawURL: "https://api.cluster.example.com#frag", + wantURL: "https://api.cluster.example.com", + }, + { + name: "path and query are both stripped", + rawURL: "https://oauth.example.com/oauth/authorize?response_type=code", + wantURL: "https://oauth.example.com", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got := openShiftAPIServerBase(tt.rawURL) + if got != tt.wantURL { + t.Fatalf("expected %q, got %q", tt.wantURL, got) + } + }) + } +} + +// TestOpenShiftLogout verifies that Logout revokes the token server-side via the +// Kubernetes OAuthAccessToken API and always returns ("", nil) — never a redirect URL. +func TestOpenShiftLogout(t *testing.T) { + t.Parallel() + + sha256Hash := func(s string) string { + hash := sha256.Sum256([]byte(s)) + return "sha256~" + b64.RawURLEncoding.EncodeToString(hash[:]) + } + + tests := []struct { + name string + token string + apiServerURL string + serverStatus int + wantDeletePath string // empty means no HTTP call expected + wantAuthHeader string + }{ + { + name: "revokes sha256~ token via Kubernetes API", + token: "sha256~testtoken", + serverStatus: http.StatusOK, + wantDeletePath: "/apis/oauth.openshift.io/v1/oauthaccesstokens/" + sha256Hash("sha256~testtoken"), + wantAuthHeader: "Bearer sha256~testtoken", + }, + { + name: "revokes legacy token via Kubernetes API", + token: "legacy-opaque-token", + serverStatus: http.StatusOK, + wantDeletePath: "/apis/oauth.openshift.io/v1/oauthaccesstokens/legacy-opaque-token", + wantAuthHeader: "Bearer legacy-opaque-token", + }, + { + name: "tolerates server error and still returns empty URL", + token: "sha256~sometoken", + serverStatus: http.StatusInternalServerError, + wantDeletePath: "/apis/oauth.openshift.io/v1/oauthaccesstokens/" + sha256Hash("sha256~sometoken"), + wantAuthHeader: "Bearer sha256~sometoken", + }, + { + name: "empty token skips revocation", + token: "", + serverStatus: http.StatusOK, + }, + { + name: "empty apiServerURL skips revocation", + token: "sha256~testtoken", + apiServerURL: "", // override: do not use server + serverStatus: http.StatusOK, + }, + { + name: "apiServerURL with path and query is normalised", + token: "legacy-token", + serverStatus: http.StatusOK, + wantDeletePath: "/apis/oauth.openshift.io/v1/oauthaccesstokens/legacy-token", + wantAuthHeader: "Bearer legacy-token", + // apiServerURL is set by the test loop to server.URL + "/oauth/authorize?foo=bar" + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + var gotMethod, gotPath, gotAuth string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotMethod = r.Method + gotPath = r.URL.Path + gotAuth = r.Header.Get("Authorization") + w.WriteHeader(tt.serverStatus) + })) + defer server.Close() + + // Determine which apiServerURL to use + apiServerURL := server.URL + switch tt.name { + case "empty apiServerURL skips revocation": + apiServerURL = "" + case "apiServerURL with path and query is normalised": + apiServerURL = server.URL + "/oauth/authorize?foo=bar" + } + + handler := &OpenShiftAuthHandler{apiServerURL: apiServerURL} + gotURL, err := handler.Logout(tt.token, "https://ui.example.com") + + if err != nil { + t.Fatalf("expected no error, got: %v", err) + } + if gotURL != "" { + t.Fatalf("expected empty redirect URL, got %q", gotURL) + } + + if tt.wantDeletePath == "" { + // No HTTP call should have been made + if gotMethod != "" { + t.Fatalf("expected no HTTP call, but got %s %s", gotMethod, gotPath) + } + return + } + + if gotMethod != http.MethodDelete { + t.Fatalf("expected DELETE request, got %q", gotMethod) + } + if gotPath != tt.wantDeletePath { + t.Fatalf("expected path %q, got %q", tt.wantDeletePath, gotPath) + } + if gotAuth != tt.wantAuthHeader { + t.Fatalf("expected Authorization %q, got %q", tt.wantAuthHeader, gotAuth) + } + }) + } +} diff --git a/proxy/auth/redirect_test.go b/proxy/auth/redirect_test.go index 5a556da44..a31d2cb74 100644 --- a/proxy/auth/redirect_test.go +++ b/proxy/auth/redirect_test.go @@ -15,7 +15,7 @@ func TestRedirectBaseMatchesRequest_NormalizesDefaultHTTPSPort(t *testing.T) { r.Host = "ui.example.com:443" u := &url.URL{Scheme: "https", Host: "ui.example.com"} - if err := redirectBaseMatchesRequest(r, u); err != nil { + if err := redirectBaseMatchesRequest(t, r, u); err != nil { t.Fatalf("expected origins to match after normalization, got error: %v", err) } } @@ -27,7 +27,7 @@ func TestRedirectBaseMatchesRequest_NormalizesDefaultHTTPPort(t *testing.T) { r.Host = "ui.example.com:80" u := &url.URL{Scheme: "http", Host: "ui.example.com"} - if err := redirectBaseMatchesRequest(r, u); err != nil { + if err := redirectBaseMatchesRequest(t, r, u); err != nil { t.Fatalf("expected origins to match after normalization, got error: %v", err) } } @@ -40,7 +40,7 @@ func TestRedirectBaseMatchesRequest_RejectsNonDefaultPortMismatch(t *testing.T) r.Host = "ui.example.com:8443" u := &url.URL{Scheme: "https", Host: "ui.example.com"} - if err := redirectBaseMatchesRequest(r, u); err == nil { + if err := redirectBaseMatchesRequest(t, r, u); err == nil { t.Fatal("expected non-default port mismatch to be rejected") } }