Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add CAM token SSO support #51

Merged
merged 16 commits into from
Feb 2, 2024
Merged

Add CAM token SSO support #51

merged 16 commits into from
Feb 2, 2024

Conversation

skovati
Copy link
Contributor

@skovati skovati commented Dec 8, 2023

closes NASA-AMMOS/aerie#1100

This PR adds two new endpoints to the Gateway, /auth/validateSSO and /auth/loginSSO, which accept SSO token cookies in order to support external auth providers. The corresponding UI PR (NASA-AMMOS/aerie-ui#1053) uses these new routes in order to support SSO login with CAM while remaining auth provider agnostic.

External auth providers

This PR also refactors our auth logic into individual modules, which are loaded at runtime based on environment variables. This means we can write auth adapters for any number of external auth providers (e.g. Auth0, AWS Cognito, etc). An adapter for CAM auth is provided in this PR.

Testing steps

This PR adds two new environment variables:

  • AUTH_SSO_TOKEN_NAME which is the name of the SSO token the Gateway should parse for in the cookies passed to either /auth/validateSSO or /auth/loginSSO. This is done so that the UI can simply forward all cookies to the Gateway, instead of needing to know specifically which cookies to serialize (and therefore need to know which auth provider we're using)

  • AUTH_UI_URL which is a URL that the Gateway will return in responses to /auth/validateSSO, so the UI will know where to redirect the user if there isn't a valid SSO token.

  • Add the following line to /etc/hosts to emulate DNS:

    • 127.0.0.1 localhost.jpl.nasa.gov
  • Pull and npm run build this PR locally

  • Pull and npm run dev corresponding UI PR locally

  • Checkout the testing/add-sso-test-config aerie branch locally, which includes new env vars as well as points to a local gateway build instead of aerie-gateway:develop

  • Run docker compose up -d --build

  • Go to localhost.jpl.nasa.gov:3000 in the browser.

Possible auth configurations

  • No auth
    • User is forwarded to Aerie UI /login page if no valid Aerie JWT is found. Validity of username / password is never checked; automatically signed in as username with admin privileges.
      • AUTH_TYPE: none
      • AUTH_UI_URL: https://localhost.jpl.nasa.gov/login
  • CAM UI auth
    • User is forwarded to CAM UI page if no valid Aerie JWT or CAM SSO token is found.
      • AUTH_TYPE: cam
      • AUTH_UI_URL: https://atb-ocio-12b.jpl.nasa.gov:8443/cam-ui
      • AUTH_URL: https://atb-ocio-12b.jpl.nasa.gov:8443/cam-api
      • AUTH_SSO_TOKEN_NAME: iPlanetDirectoryPro
  • CAM API auth + Aerie login UI
    • User is forwarded to Aerie UI /login page if no valid Aerie JWT is found. Credentials passed in here will be tested against the CAM API for validity.
      • AUTH_TYPE: cam
      • AUTH_UI_URL: https://aerie.test.jpl.nasa.gov/login
      • AUTH_URL: https://atb-ocio-12b.jpl.nasa.gov:8443/cam-api

Manual testing of all three configuration options has been completed.

@Mythicaeda
Copy link
Contributor

RE: AUTH_SSO_TOKEN_NAME, do we know if it's always going to be a single cookie or should we let this be a list?

@skovati
Copy link
Contributor Author

skovati commented Dec 8, 2023

@Mythicaeda Good question.

The long term plan is to allow the gateway to use some sort of "auth adapter" that will actually implement these functions (so we can support arbitrary auth providers, and users can even write their own). In that case, Aerie operators could set this env var to e.g. TokenName;Signature and then have the custom auth adapter string.split(';') to extract multiple cookie names, if that use case does arise.

@Mythicaeda
Copy link
Contributor

Sure, although if it's an array from the get-go there's no need for a custom adapter to extract this extra information.

Additionally, I'm not confident that the TokenName;Signature, split on ; approach will work, as the Gateway will try to grab the cookie TokenName;Signature to send to the external auth service, but TokenName;Signature doesn't exist. It instead needs to grab and send along both the TokenName and Signature cookies

@duranb
Copy link
Contributor

duranb commented Dec 11, 2023

@Mythicaeda the adapters won't necessarily just be for parsing cookies. They will be in charge of formatting the request to the auth that they interface with and then formatting a response back to the UI that implements an aerie-gateway <-> aerie-ui interface.

Also, as an aside, ; can't be used in cookies unless it's first encoded in some way because that's the delimeter used to split cookies.

Copy link
Contributor

@duranb duranb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's almost there I think! I'm actually thinking that we should just go forward with the explicit auth adapters in this PR because it's practically 95% there. I'm down to huddle again to explain my thoughts if you were interested.

src/env.ts Show resolved Hide resolved
src/env.ts Show resolved Hide resolved
src/packages/auth/functions.ts Outdated Show resolved Hide resolved
src/packages/auth/functions.ts Outdated Show resolved Hide resolved
src/packages/files/files.ts Outdated Show resolved Hide resolved
src/packages/files/files.ts Outdated Show resolved Hide resolved
src/packages/auth/routes.ts Outdated Show resolved Hide resolved
src/packages/auth/routes.ts Outdated Show resolved Hide resolved
@skovati
Copy link
Contributor Author

skovati commented Dec 11, 2023

@Mythicaeda

re:

as the Gateway will try to grab the cookie TokenName;Signature to send to the external auth service, but TokenName;Signature doesn't exist.

As @duranb mentioned, the custom adapters would be responsible for extracting cookie values as well as formatting and making requests to some custom auth provider, so they would know ahead of time to parse AUTH_SSO_TOKEN_NAME as a list of names instead of a single name.

As you eluded to, the current code doesn't support this as it's explicitly grabbing cookies in the auth endpoinds, but this logic would be moved into custom auth adapters when we introduce that feature (which may be in this PR)

Copy link
Contributor

@duranb duranb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looking good! Just a couple more comments.

src/packages/auth/adapters/DefaultAuthAdapter.ts Outdated Show resolved Hide resolved
src/main.ts Show resolved Hide resolved
@camargo
Copy link
Member

camargo commented Dec 14, 2023

Nice!

src/main.ts Outdated Show resolved Hide resolved
src/packages/auth/functions.ts Show resolved Hide resolved
src/packages/auth/routes.ts Show resolved Hide resolved
src/packages/auth/types.ts Show resolved Hide resolved
src/packages/auth/adapters/NoAuthAdapter.ts Outdated Show resolved Hide resolved
src/packages/auth/adapters/FakeAuthAdapter.ts Outdated Show resolved Hide resolved
@Mythicaeda
Copy link
Contributor

re:

as the Gateway will try to grab the cookie TokenName;Signature to send to the external auth service, but TokenName;Signature doesn't exist.

As duranb mentioned, the custom adapters would be responsible for extracting cookie values as well as formatting and making requests to some custom auth provider, so they would know ahead of time to parse AUTH_SSO_TOKEN_NAME as a list of names instead of a single name.

As you eluded to, the current code doesn't support this as it's explicitly grabbing cookies in the auth endpoinds, but this logic would be moved into custom auth adapters when we introduce that feature (which may be in this PR)

Having seen sample adapters, I understand your what you meant with having the adapter code fetch the cookies. However, I stand by that making the adapter code parse AUTH_SSO_TOKEN_NAME into a list is unnecessarily inflexible.

@skovati
Copy link
Contributor Author

skovati commented Dec 20, 2023

@Mythicaeda I would argue that letting the adapters parse cookies however they want is the most flexible option.

@skovati skovati requested review from a team as code owners December 27, 2023 19:39
@skovati skovati force-pushed the feature/cam-token-sso branch 2 times, most recently from af1a283 to de15ba5 Compare December 27, 2023 22:39
@skovati skovati removed the request for review from dandelany January 3, 2024 19:47
@skovati skovati removed the request for review from joswig January 3, 2024 19:47
@skovati skovati removed request for a team January 3, 2024 20:28
docs/ENVIRONMENT.md Outdated Show resolved Hide resolved
src/packages/auth/functions.ts Outdated Show resolved Hide resolved
src/packages/auth/types.ts Show resolved Hide resolved
Copy link
Contributor

@Mythicaeda Mythicaeda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@skovati skovati merged commit 35a30ed into develop Feb 2, 2024
3 checks passed
@skovati skovati deleted the feature/cam-token-sso branch February 2, 2024 18:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Support for SSO with CAM
4 participants