diff --git a/src/core/components/auth/auths.jsx b/src/core/components/auth/auths.jsx index a29b57359f3..b5b97787931 100644 --- a/src/core/components/auth/auths.jsx +++ b/src/core/components/auth/auths.jsx @@ -35,9 +35,13 @@ export default class Auths extends React.Component { e.preventDefault() let { authActions, definitions } = this.props - let auths = definitions.map( (val, key) => { - return key - }).toArray() + // `definitions` is an Immutable.Map whose keys are the security scheme + // names we want to log out of. Use `keySeq().toArray()` rather than + // `.map((val, key) => key).toArray()` because `Map#toArray()` returns + // `[key, value]` pairs under Immutable v4+ and values-only under v3, + // so the latter pattern silently produces `[[name, schema]]` instead + // of `[name]` when the host bundles a newer Immutable (see #10761). + let auths = definitions.keySeq().toArray() this.setState(auths.reduce((prev, auth) => { prev[auth] = "" diff --git a/src/core/plugins/auth/wrap-actions.js b/src/core/plugins/auth/wrap-actions.js index b7f12b342dc..ff478c067b6 100644 --- a/src/core/plugins/auth/wrap-actions.js +++ b/src/core/plugins/auth/wrap-actions.js @@ -1,7 +1,7 @@ /** * @prettier */ -import { fromJS } from "immutable" +import { fromJS, Map } from "immutable" /** * `authorize` and `logout` wrapped actions provide capacity @@ -44,7 +44,12 @@ export const logout = (oriAction, system) => (payload) => { try { if (configs.persistAuthorization && Array.isArray(payload)) { payload.forEach((authorizedName) => { - const auth = authorized.get(authorizedName, {}) + // Defend against missing entries in the `authorized` store — e.g. + // when a name was never authorized, or when an upstream caller + // passes an unexpected key shape. Use an empty Immutable Map() so + // the following getIn() calls are always safe; plain-object + // fallbacks crash here because getIn is Immutable-specific. + const auth = authorized.get(authorizedName, Map()) const isApiKeyAuth = auth.getIn(["schema", "type"]) === "apiKey" const isInCookie = auth.getIn(["schema", "in"]) === "cookie" const isApiKeyInCookie = isApiKeyAuth && isInCookie diff --git a/src/core/plugins/oas31/components/auth/auths.jsx b/src/core/plugins/oas31/components/auth/auths.jsx index 227d5656c46..da4e6775838 100644 --- a/src/core/plugins/oas31/components/auth/auths.jsx +++ b/src/core/plugins/oas31/components/auth/auths.jsx @@ -38,11 +38,13 @@ class Auths extends React.Component { e.preventDefault() let { authActions, definitions } = this.props - let auths = definitions - .map((val, key) => { - return key - }) - .toArray() + // `definitions` is an Immutable.Map whose keys are the security scheme + // names we want to log out of. Use `keySeq().toArray()` rather than + // `.map((val, key) => key).toArray()` because `Map#toArray()` returns + // `[key, value]` pairs under Immutable v4+ and values-only under v3, + // so the latter pattern silently produces `[[name, schema]]` instead + // of `[name]` when the host bundles a newer Immutable (see #10761). + let auths = definitions.keySeq().toArray() this.setState( auths.reduce((prev, auth) => { diff --git a/test/unit/core/components/auth/auths.jsx b/test/unit/core/components/auth/auths.jsx new file mode 100644 index 00000000000..7dc0d0d17c3 --- /dev/null +++ b/test/unit/core/components/auth/auths.jsx @@ -0,0 +1,85 @@ +/** + * @prettier + */ +import React from "react" +import { shallow } from "enzyme" +import { fromJS, Map } from "immutable" + +import Auths from "core/components/auth/auths" + +describe(" logoutClick", function () { + const dummyComponent = () => null + const components = { + AuthItem: dummyComponent, + oauth2: dummyComponent, + Button: dummyComponent, + } + const getComponentStub = (name) => components[name] || dummyComponent + + const buildProps = (definitions) => ({ + definitions, + getComponent: getComponentStub, + authSelectors: { + authorized: () => Map(), + }, + authActions: { + logoutWithPersistOption: jest.fn(), + }, + errSelectors: {}, + specSelectors: {}, + }) + + it("sends only the scheme names to logoutWithPersistOption for a single-scheme map", function () { + // The Authorize popup renders one per requirement group, + // passing an Immutable.Map whose keys are the scheme names. The + // Logout handler must emit those keys as a flat array of strings. + const definitions = fromJS({ + ApiKeyAuth: { type: "apiKey", in: "header", name: "X-API-Key" }, + }) + const props = buildProps(definitions) + const wrapper = shallow() + + wrapper.instance().logoutClick({ preventDefault: () => {} }) + + expect(props.authActions.logoutWithPersistOption).toHaveBeenCalledWith([ + "ApiKeyAuth", + ]) + }) + + it("sends all scheme names when the map has several entries", function () { + const definitions = fromJS({ + ApiKeyAuth: { type: "apiKey", in: "header", name: "X-API-Key" }, + BearerAuth: { type: "http", scheme: "bearer" }, + }) + const props = buildProps(definitions) + const wrapper = shallow() + + wrapper.instance().logoutClick({ preventDefault: () => {} }) + + expect(props.authActions.logoutWithPersistOption).toHaveBeenCalledWith([ + "ApiKeyAuth", + "BearerAuth", + ]) + }) + + it("does not leak [key, value] pairs into the payload (guard against Immutable v4 toArray semantics)", function () { + // `Map#toArray()` switched in Immutable v4 from values-only to + // `[key, value]` pairs. The previous `.map((v, k) => k).toArray()` + // pattern therefore emitted `[[name, schema]]` when a host bundled + // Immutable v4+, which broke logout (see #10761). Guard the flat- + // string shape regardless of Immutable version. + const definitions = fromJS({ + ApiKeyAuth: { type: "apiKey", in: "header", name: "X-API-Key" }, + }) + const props = buildProps(definitions) + const wrapper = shallow() + + wrapper.instance().logoutClick({ preventDefault: () => {} }) + + const [payload] = props.authActions.logoutWithPersistOption.mock.calls[0] + expect(Array.isArray(payload)).toBe(true) + expect(payload).toHaveLength(1) + expect(typeof payload[0]).toBe("string") + expect(payload[0]).toBe("ApiKeyAuth") + }) +}) diff --git a/test/unit/core/plugins/auth/wrap-actions.js b/test/unit/core/plugins/auth/wrap-actions.js index f9f3b98575c..38f90f27ea5 100644 --- a/test/unit/core/plugins/auth/wrap-actions.js +++ b/test/unit/core/plugins/auth/wrap-actions.js @@ -90,6 +90,57 @@ describe("Cookie based apiKey persistence in document.cookie", () => { expect(document.cookie).toEqual("apiKeyCookie=; Max-Age=-99999999") }) + + it("should delete cookie even when payload contains names missing from the authorized store", () => { + // Defensive: if `authorized.get(name)` can't find an entry, the + // fallback must expose `.getIn(...)` safely. A plain-object + // fallback crashes here; an empty Immutable Map() keeps the + // iteration going and still clears cookies for names that are + // present. Guards the fix for + // https://github.com/swagger-api/swagger-ui/issues/10761. + const authorized = fromJS({ + api_key: { + schema: { + type: "apiKey", + name: "apiKeyCookie", + in: "cookie", + }, + value: "test", + }, + }) + const system = { + getConfigs: () => ({ + persistAuthorization: true, + }), + authSelectors: { + authorized: () => authorized, + }, + } + const oriAction = jest.fn() + + const logoutAction = () => + logout(oriAction, system)(["missing_auth", "api_key"]) + + expect(logoutAction).not.toThrow() + expect(document.cookie).toEqual("apiKeyCookie=; Max-Age=-99999999") + expect(oriAction).toHaveBeenCalledWith(["missing_auth", "api_key"]) + }) + + it("should not throw when every name in payload is missing from the authorized store", () => { + const system = { + getConfigs: () => ({ + persistAuthorization: true, + }), + authSelectors: { + authorized: () => fromJS({}), + }, + } + + const logoutAction = () => logout(jest.fn(), system)(["missing_auth"]) + + expect(logoutAction).not.toThrow() + expect(document.cookie).toEqual("") + }) }) describe("given persistAuthorization=false", () => {