Fixes anonymous auth flow to work with SAML #1817

DarshitChanpura · 2024-03-06T04:06:33Z

Description

This PR fixes 2 things:

Anonymous login button would not show if only 1 auth option was supplied
Broken SAML auth flow when anonymous auth is enabled.

Why these changes are required?

For SAML and anonymous auth options to function when both options are enabled

What is the old behavior before changes and new behavior after changes?

At present, SAML auth flow is broken since anonymous auth automatically assumes the Anonymous user when no auth credentials are found. See security plugin PR[1] for more details.

[1] - Companion PR: opensearch-project/security#4097

Issues Resolved

Resolves [BUG] Having Anonymous and SAML as sign in options does not work. SAML Login fails when Anonymous is enabled #1731

Testing

Automated + Manual

Check List

New functionality includes testing
New functionality has been documented
Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Darshit Chanpura <[email protected]>

DarshitChanpura · 2024-03-06T04:11:27Z

server/auth/types/basic/basic_auth.ts

-      if (this.config.auth.anonymous_auth_enabled) {
-        const redirectLocation = `${this.coreSetup.http.basePath.serverBasePath}${ANONYMOUS_AUTH_LOGIN}?${nextUrlParam}`;
-        return response.redirected({
-          headers: {
-            location: `${redirectLocation}`,
-          },
-        });
-      } else {


This if block caused all logout requests to be redirected to the anonymous login page when anonymous auth is enabled. This is incorrect. All logout requests should redirect to login page. (Similar change below for multi_auth.ts)

server/auth/types/basic/routes.ts

Signed-off-by: Darshit Chanpura <[email protected]>

public/apps/login/login-page.tsx

DarshitChanpura · 2024-03-11T22:08:05Z

server/auth/types/basic/routes.ts

-            user = await this.securityClient.authenticateWithHeaders(request, {});
+            user = await this.securityClient.authenticateWithHeaders(request, {
+              authorization: ANONYMOUS_AUTH_HEADER,
+            });


Out of scope of this PR, but code between lines 214-228 is dead-code because multi_tenancy is never set upon receiving auth response from backend.

The dead-code:

security-dashboards-plugin/server/auth/types/basic/routes.ts

Lines 214 to 227 in 3c0c2f9

if (user.multitenancy_enabled) {

const selectTenant = resolveTenant({

request,

username: user.username,

roles: user.roles,

availableTenants: user.tenants,

config: this.config,

cookie: sessionStorage,

multitenancyEnabled: user.multitenancy_enabled,

privateTenantEnabled: user.private_tenant_enabled,

defaultTenant: user.default_tenant,

});

sessionStorage.tenant = selectTenant;

}

this is the reason:

security-dashboards-plugin/server/backend/opensearch_security_client.ts

Lines 98 to 104 in 3c0c2f9

return {

username: esResponse.user_name,

roles: esResponse.roles,

backendRoles: esResponse.backend_roles,

tenants: esResponse.tenants,

selectedTenant: esResponse.user_requested_tenant,

};

as we can see that the property multi_tenancy for user object is never set, hence the if condition is never true.

Signed-off-by: Darshit Chanpura <[email protected]>

…orrectly Signed-off-by: Darshit Chanpura <[email protected]>

codecov · 2024-03-11T23:15:08Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 67.51%. Comparing base (c80b64a) to head (d682701).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1817      +/-   ##
==========================================
+ Coverage   67.38%   67.51%   +0.12%     
==========================================
  Files          94       94              
  Lines        2410     2410              
  Branches      320      327       +7     
==========================================
+ Hits         1624     1627       +3     
+ Misses        708      706       -2     
+ Partials       78       77       -1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: Darshit Chanpura <[email protected]>

…ion header Signed-off-by: Darshit Chanpura <[email protected]>

DarshitChanpura · 2024-03-12T04:46:28Z

Integration tests will pass once opensearch-project/security#4097 is merged.

Expand to see local (successful) run of `basic_auth.test.ts` with updated backend:

➜  security-dashboards-plugin git:(anonymous-saml-bug-fix) ✗ yarn test:jest_server -- basic_auth.test.ts
yarn run v1.22.19
warning From Yarn 1.0 onwards, scripts don't require "--" for options to be forwarded. In a future version, any explicit "--" will be forwarded as-is to the scripts.
$ node ./test/jest_integration/runIdpServer.js &
$ node ./test/run_jest_tests.js --config ./test/jest.config.server.js basic_auth.test.ts
Listener Port:
  localhost:7000
HTTPS Enabled:
  false

[Identity Provider]

Issuer URI:
  urn:example:idp
Sign Response Message:
  true
Encrypt Assertion:
  false
Authentication Context Class Reference:
  urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport
Authentication Context Declaration:
  None
Default RelayState:
  None

[Service Provider]

Issuer URI:
  None
Audience URI:
  https://localhost:9200
ACS URL:
  http://localhost:5601/_opendistro/_security/saml/acs
SLO URL:
  None
Trust ACS URL in Request:
  true
  
Starting IdP server on port localhost:7000...

node:events:495
      throw er; // Unhandled 'error' event
      ^

Error: listen EADDRINUSE: address already in use ::1:7000
    at Server.setupListenHandle [as _listen2] (node:net:1817:16)
    at listenInCluster (node:net:1865:12)
    at GetAddrInfoReqWrap.doListen [as callback] (node:net:2014:7)
    at GetAddrInfoReqWrap.onlookup [as oncomplete] (node:dns:109:8)
Emitted 'error' event on Server instance at:
    at emitErrorNT (node:net:1844:8)
    at process.processTicksAndRejections (node:internal/process/task_queues:82:21) {
  code: 'EADDRINUSE',
  errno: -48,
  syscall: 'listen',
  address: '::1',
  port: 7000
}

Node.js v18.19.0
ciGroups []
  console.log
    [agentkeepalive:deprecated] options.freeSocketKeepAliveTimeout is deprecated, please use options.freeSocketTimeout instead

      at deprecate (node_modules/agentkeepalive/lib/agent.js:30:11)

  console.log
    [agentkeepalive:deprecated] options.freeSocketKeepAliveTimeout is deprecated, please use options.freeSocketTimeout instead

      at deprecate (node_modules/agentkeepalive/lib/agent.js:30:11)

  console.log
    Starting OpenSearchDashboards server..

      at Object.<anonymous> (plugins/security-dashboards-plugin/test/jest_integration/basic_auth.test.ts:89:13)

  console.log
    [agentkeepalive:deprecated] options.freeSocketKeepAliveTimeout is deprecated, please use options.freeSocketTimeout instead

      at deprecate (node_modules/agentkeepalive/lib/agent.js:30:11)

  console.log
    [agentkeepalive:deprecated] options.freeSocketKeepAliveTimeout is deprecated, please use options.freeSocketTimeout instead

      at deprecate (node_modules/agentkeepalive/lib/agent.js:30:11)

  console.log
    Started OpenSearchDashboards server

      at Object.<anonymous> (plugins/security-dashboards-plugin/test/jest_integration/basic_auth.test.ts:92:13)

  console.log
    Error: Response Error: 403 Forbidden
        at Object.<anonymous>.internals.Client._shortcut (/Users/XXXXX/OpenSearch-Dashboards/plugins/security-dashboards-plugin/node_modules/@hapi/wreck/lib/index.js:569:15)
        at processTicksAndRejections (node:internal/process/task_queues:95:5)
        at Object.<anonymous> (/Users/XXXXX/OpenSearch-Dashboards/plugins/security-dashboards-plugin/test/jest_integration/basic_auth.test.ts:108:36) {
      data: {
        isResponseError: true,
        headers: {
          'x-opensearch-version': 'OpenSearch/3.0.0 (opensearch)',
          'content-type': 'application/json; charset=UTF-8',
          'content-length': '48'
        },
        res: IncomingMessage {
          _readableState: [ReadableState],
          _events: [Object: null prototype],
          _eventsCount: 2,
          _maxListeners: undefined,
          socket: [TLSSocket],
          httpVersionMajor: 1,
          httpVersionMinor: 1,
          httpVersion: '1.1',
          complete: true,
          rawHeaders: [Array],
          rawTrailers: [],
          joinDuplicateHeaders: undefined,
          aborted: false,
          upgrade: false,
          url: '',
          method: null,
          statusCode: 403,
          statusMessage: 'Forbidden',
          client: [TLSSocket],
          _consuming: false,
          _dumped: false,
          req: [ClientRequest],
          [Symbol(kCapture)]: false,
          [Symbol(kHeaders)]: [Object],
          [Symbol(kHeadersCount)]: 6,
          [Symbol(kTrailers)]: null,
          [Symbol(kTrailersCount)]: 0
        },
        payload: <Buffer 7b 22 73 74 61 74 75 73 22 3a 22 46 4f 52 42 49 44 44 45 4e 22 2c 22 6d 65 73 73 61 67 65 22 3a 22 41 63 63 65 73 73 20 64 65 6e 69 65 64 22 7d>
      },
      isBoom: true,
      isServer: false,
      output: {
        statusCode: 403,
        payload: {
          statusCode: 403,
          error: 'Forbidden',
          message: 'Response Error: 403 Forbidden'
        },
        headers: {}
      }
    }

      at Object.<anonymous> (plugins/security-dashboards-plugin/test/jest_integration/basic_auth.test.ts:120:15)

  console.log
    [agentkeepalive:deprecated] options.freeSocketKeepAliveTimeout is deprecated, please use options.freeSocketTimeout instead

      at deprecate (node_modules/agentkeepalive/lib/agent.js:30:11)

 PASS  test/jest_integration/basic_auth.test.ts (5.695 s)
  start OpenSearch Dashboards server
    ✓ can access login page without credentials (32 ms)
    ✓ cannot access home page as anonymous user if no credentials are supplied (30 ms)
    ✓ can access home page as anonymous user (46 ms)
    ✓ call authinfo API as admin (12 ms)
    ✓ call authinfo API without credentials (3 ms)
    ✓ call authinfo API as anonymous user (7 ms)
    ✓ call authinfo API with cookie (17 ms)
    ✓ login with user name and password (9 ms)
    ✓ login fails with invalid credentials (307 ms)
    ✓ log out (14 ms)
    ✓ anonymous auth (4 ms)
    ✓ anonymous disabled (4 ms)
    ✓ enforce authentication on api/status route (317 ms)
    ✓ can access api/status route with admin credential (12 ms)
    ✓ redirect for home follows login for anonymous auth enabled (10 ms)
    ✓ redirect for home follows login for anonymous auth disabled (8 ms)
    ✓ redirects to an object ignores after hash with anonymous auth enabled (9 ms)

 PASS  server/auth/types/basic/basic_auth.test.ts
  Basic auth tests
    ✓ getKeepAliveExpiry (5 ms)

Test Suites: 2 passed, 2 total
Tests:       18 passed, 18 total
Snapshots:   0 total
Time:        6.482 s, estimated 9 s
Ran all test suites matching /basic_auth.test.ts/i.
✨  Done in 8.74s.
➜  security-dashboards-plugin git:(anonymous-saml-bug-fix) ✗

Signed-off-by: Darshit Chanpura <[email protected]>

DarshitChanpura · 2024-03-21T01:37:59Z

This effort is continued in #1839. Closing this out.

Fixes anonymous auth flow to work with SAML

199683c

Signed-off-by: Darshit Chanpura <[email protected]>

DarshitChanpura mentioned this pull request Mar 6, 2024

Skips authentication only for anonymous auth requests when anonymous-auth is enabled opensearch-project/security#4097

Closed

2 tasks

DarshitChanpura commented Mar 6, 2024

View reviewed changes

DarshitChanpura added 3 commits March 11, 2024 16:40

Adds hardcoded credentials for anonymous user

cbdf7a6

Signed-off-by: Darshit Chanpura <[email protected]>

Updates basic auth header to be a config constant

a0f2db4

Signed-off-by: Darshit Chanpura <[email protected]>

Removes unneeded usage of anonymous auth header constant

3c0c2f9

Signed-off-by: Darshit Chanpura <[email protected]>

DarshitChanpura commented Mar 11, 2024

View reviewed changes

DarshitChanpura added 2 commits March 11, 2024 18:41

Updates logic to display anonymous auth login button

8322a23

Signed-off-by: Darshit Chanpura <[email protected]>

Adds test to check whether anonymous auth login button is displayed c…

f331377

…orrectly Signed-off-by: Darshit Chanpura <[email protected]>

DarshitChanpura added 2 commits March 11, 2024 23:57

Fixes integrationtests

63abb41

Signed-off-by: Darshit Chanpura <[email protected]>

Adds integration tests for anonymous auth login with basic authorizat…

5492d2a

…ion header Signed-off-by: Darshit Chanpura <[email protected]>

Generates random password for anonymous user

9d47261

Signed-off-by: Darshit Chanpura <[email protected]>

DarshitChanpura changed the title ~~[WIP] Fixes anonymous auth flow to work with SAML~~ Fixes anonymous auth flow to work with SAML Mar 12, 2024

DarshitChanpura marked this pull request as ready for review March 12, 2024 04:49

DarshitChanpura requested review from cliu123, cwperks, davidlago, peternied, RyanL1997 and stephen-crawford as code owners March 12, 2024 04:49

DarshitChanpura added the backport 2.x backport to 2.x branch label Mar 12, 2024

Fixes lint errors

d682701

Signed-off-by: Darshit Chanpura <[email protected]>

DarshitChanpura closed this Mar 21, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes anonymous auth flow to work with SAML #1817

Fixes anonymous auth flow to work with SAML #1817

DarshitChanpura commented Mar 6, 2024 •

edited

Loading

DarshitChanpura Mar 6, 2024

DarshitChanpura Mar 11, 2024 •

edited

Loading

codecov bot commented Mar 11, 2024 •

edited

Loading

DarshitChanpura commented Mar 12, 2024 •

edited

Loading

DarshitChanpura commented Mar 21, 2024

	if (user.multitenancy_enabled) {
	const selectTenant = resolveTenant({
	request,
	username: user.username,
	roles: user.roles,
	availableTenants: user.tenants,
	config: this.config,
	cookie: sessionStorage,
	multitenancyEnabled: user.multitenancy_enabled,
	privateTenantEnabled: user.private_tenant_enabled,
	defaultTenant: user.default_tenant,
	});
	sessionStorage.tenant = selectTenant;
	}

	return {
	username: esResponse.user_name,
	roles: esResponse.roles,
	backendRoles: esResponse.backend_roles,
	tenants: esResponse.tenants,
	selectedTenant: esResponse.user_requested_tenant,
	};

Fixes anonymous auth flow to work with SAML #1817

Fixes anonymous auth flow to work with SAML #1817

Conversation

DarshitChanpura commented Mar 6, 2024 • edited Loading

Description

Category

Why these changes are required?

What is the old behavior before changes and new behavior after changes?

Issues Resolved

Testing

Check List

DarshitChanpura Mar 6, 2024

Choose a reason for hiding this comment

DarshitChanpura Mar 11, 2024 • edited Loading

Choose a reason for hiding this comment

codecov bot commented Mar 11, 2024 • edited Loading

Codecov Report

DarshitChanpura commented Mar 12, 2024 • edited Loading

DarshitChanpura commented Mar 21, 2024

DarshitChanpura commented Mar 6, 2024 •

edited

Loading

DarshitChanpura Mar 11, 2024 •

edited

Loading

codecov bot commented Mar 11, 2024 •

edited

Loading

DarshitChanpura commented Mar 12, 2024 •

edited

Loading