Skip to content

feat: token exchange as part of VSC login #1240

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

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

feat: token exchange as part of VSC login #1240

wants to merge 10 commits into from

Conversation

heyzec
Copy link

@heyzec heyzec commented Mar 24, 2025

FYP: Source Academy as Visual Studio Code Extension

The Visual Studio Code extension for Source Academy requires a slight change to the login flow.

Idea

Here is the big picture of why this PR is needed, as well as why it is as such.

  1. IFrames

    • Originally: After a normal SAML login, the backend redirects to the a frontend route (/login/callback)
    • Constraint: The NUS login page cannot be displayed within the VSC as an iframe, instead requiring login via external browser. From the backend's perspective, this means the login request is no longer the same device as the client.
    • Idea: Backend behaviour has to be modified to redirect to a VSC deeplink instead.
  2. Security

    • Originally: After a normal SAML login, backend passes auth tokens to frontend indirectly via cookies.
    • Idea 1: Via query parameters in the deeplink, we pass auth tokens to VSC extension for it to pass it to the iframe frontend.
    • Constraint: We don't want the VSC extension to have knowledge of auth (access and refresh) tokens.
    • Final idea: Pass a temp token/code via the deeplink. The iframe frontend then uses temp token to exchange for the real auth tokens.

Modified login flow for VSC

  1. User runs the "Login" command from within VSC, opening a browser window

  2. Normal SAML flow

    • Backend (service provider): api.sourceacademy.nus.edu.sg/sso/auth/signin/student?target_url=<url in (3)>
    • NUS (identity provider): vafs.u.nus.edu
    • Backend (service provider): api.sourceacademy.nus.edu.sg/sso/sp/consume/...
    • Backend redirects browser to another backend endpoint
  3. New backend endpoints and behaviour

    • api.sourceacademy.nus.edu.sg/auth/saml_redirect_vscode?provider=<provider>
    • A random temp code is generated and stored into a new token_exchange table with 1 minute expiry
    • Server redirects user to a VSC deeplink, see vscode_redirect_url_prefix in app vars (vscode://source-academy.source-academy/sso?code=<code>&provider=<provider>)
  4. User's VSC extension handles and opens the iframe

    • URL pointing to backend at api.sourceacademy.nus.edu.sg/auth/exchange?code=<code>&provider=<provider>
    • Server validates code against table before generated tokens (both access and refresh)
    • Server redirects user to a new frontend route sourceacademy.nus.edu.sg/login/vscode_callback?access_token=<access_token>&refresh_token=<refresh_token>. This route saves tokens into Redux store and redirects users to /welcome.

@heyzec heyzec requested a review from RichDom2185 March 24, 2025 16:11
@heyzec heyzec self-assigned this Mar 24, 2025
@heyzec
Copy link
Author

heyzec commented Mar 24, 2025

This is my first time writing elixir code 😅

@heyzec heyzec changed the title Create flow for token exchange needed for VSC extension feat: token exchange as part of VSC login Mar 24, 2025
@coveralls
Copy link

coveralls commented Mar 24, 2025

Coverage Status

coverage: 92.829% (-0.8%) from 93.607%
when pulling 600b290 on vscode/auth
into bd37d04 on master.

Copy link
Contributor

@GabrielCWT GabrielCWT left a comment

Choose a reason for hiding this comment

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

LGTM, will wait for Richard to take another look at it as I'm not too familiar with authentication in SA

alias Cadet.Accounts.User

schema "token_exchange" do
field(:code, :string)
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor nit: Possibly make this a primary key?

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.

4 participants