[extension] Use dedicated audience for dust api. Add scopes verification #8719

tdraier · 2024-11-18T15:13:23Z

Description

This introduces the usage of a custom auth0 API for our public api. The audience is set in an env var , DUST_API_AUDIENCE which should be https://dust-dev.tt/api/v1 in development or https://dust.tt/api/v1 in production.

This API defines scopes in auth0 configuration, that will need to be extended, for checking particular endpoints in the api. All scopes used by the app are set - we'll have to add/define other ones in another PR.

The scopes required by the app must be specified when requesting authorization, which will result in displaying a consent screen :

(This consent screen can be skipped for 1st party apps , i.e. our own apps registered in our auth0 tenant, but must be shown for 3rd party apps)

The 2 wrappers withPublicAPIAuthentication and withAuth0TokenAuthentication are extended to support new option "resourceName" that can be used to create the scope.

Scope are only checked against oauth tokens . sk- keys are considered system and do not include any scope (no scope check).

Risk

For oauth users, some routes now require scope.

Deploy Plan

Set up prod API on auth0
Set DUST_API_AUDIENCE env vars
Deploy front

github-actions · 2024-11-18T15:15:10Z

	Warnings
⚠️	Files in `**/api/v1/` have been modified and the PR has the `documentation-ack` label. Don't forget to run `npm run docs` and use the `Deploy OpenAPI Docs` Github action to update https://docs.dust.tt/reference.

Generated by 🚫 dangerJS against 9490661

front/lib/api/auth0.ts

flvndvd

LGTM, left one last comment.

flvndvd · 2024-11-19T08:05:40Z

front/lib/api/auth0.ts

+  requiredScopes?: Record<string, string>
+) {
+  if (requiredScopes && req.method && requiredScopes[req.method]) {
+    return [requiredScopes[req.method]];


Why do we need to return an array here? If it's expected maybe it should be getRequiredScopes and let's define the return type.

Note: are they all required ?
Usually it's one of them, not all of them as scopes are granular.

If one of them, i would rename as getAllowedScopes()

right, I don't think we need an array for now

i was expecting all required, but yes could be an array of "at least one", that's probably more useful. For now i think we can stick with one single value

Fraggle · 2024-11-19T08:08:56Z

Love that !
~~Quick question: did you define the scopes in auth0 dev env as well ?~~ (saw that you did)
Seems like the scopes values are not type checked, can you add it if not ?

Fraggle · 2024-11-19T08:09:29Z

~~nit: from the screenshot, the user facing description of the scopes in the consent screen are not amazing, can you update ?~~ (saw that you did)

Fraggle · 2024-11-19T08:10:17Z

@tdraier but I'm wondering if you are not going to break the raycast extension.

Fraggle

I'm putting a request change because I'm worried that it breaks the raycast extension.
Let's talk before merging.

Fraggle · 2024-11-19T08:20:06Z

front/lib/api/auth0.ts

+          return resolve(new Err(Error("Invalid token payload.")));
+        }
+
+        if (requiredScopes) {


don't you need to check if the audience matches too ?

(and maybe skip the checks if not the right audience, as a temporary measure not to break the raycast extension)

The audience is checked along with the expiration and issuer by jwt.verify

I believe it's checked for consistency / signature but not for a specific value (may be wrong)

My point here is twofold:

do the checks on scope only if the audience match the expected audience

TEMPORARY, otherwise, let it go thru (so the raycast extension still works)

After the extension is updated, we'll wait a few days and then remove the temporary passthru.

( jwt.verify checks that the audience matches the one passed in parameter )

Fraggle · 2024-11-19T08:21:28Z

front/lib/api/auth0.ts

+
+export function getRequiredScope(
+  req: NextApiRequest,
+  requiredScopes?: Record<string, string>


imho, the typing of that arg should be more strict (POST|GET|PATCH|DELETE: scopes that exists)

req.method is not typed (it's a string), so in theory I can receive anything in here

...but in practice we know what we use (and we can extends if needed)

i was afraid of the typings, but ok, done 😛

Fraggle · 2024-11-19T08:21:51Z

front/lib/api/auth0.ts

+        if (requiredScopes) {
+          const availableScopes = decoded.scope.split(" ");
+          if (
+            requiredScopes.some((scope) => !availableScopes.includes(scope))


see my comment required vs allowed.

tdraier · 2024-11-19T08:34:19Z

@tdraier but I'm wondering if you are not going to break the raycast extension.

this definitely require changes in raycast extension

Fraggle · 2024-11-19T08:35:55Z

@tdraier but I'm wondering if you are not going to break the raycast extension.

this definitely require changes in raycast extension

I can do it swiftly but there is a delay for them to approve, and for users to update. We need an intermediary step where the extension still work.

Fraggle

LGTM !

tdraier added 2 commits November 18, 2024 16:01

Use dedicated audience for dust api. Add scopes verification

202f127

remove forced consent screen

0241a9c

tdraier requested review from PopDaph and flvndvd November 18, 2024 15:13

tdraier requested a review from Fraggle November 18, 2024 15:15

flvndvd reviewed Nov 18, 2024

View reviewed changes

front/lib/api/auth0.ts Outdated Show resolved Hide resolved

front/lib/api/auth0.ts Outdated Show resolved Hide resolved

front/lib/api/auth0.ts Outdated Show resolved Hide resolved

tdraier added 5 commits November 18, 2024 22:40

review comments, add scopes

663b8b7

lint

bb0dea0

lint

8a9f80f

add scopes

10a9a1a

lint

dc76e61

tdraier requested a review from flvndvd November 19, 2024 06:48

remove forced consent screen

738d1ea

flvndvd approved these changes Nov 19, 2024

View reviewed changes

Fraggle requested changes Nov 19, 2024

View reviewed changes

Fraggle reviewed Nov 19, 2024

View reviewed changes

types improvements, handle legacy audience

9490661

Fraggle self-requested a review November 19, 2024 09:16

Fraggle approved these changes Nov 19, 2024

View reviewed changes

tdraier added the documentation-ack Used to acknowledge that documentation is up-to-date with this PR label Nov 19, 2024

tdraier merged commit 217bc8b into main Nov 19, 2024
6 checks passed

tdraier deleted the oauth-api branch November 19, 2024 09:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[extension] Use dedicated audience for dust api. Add scopes verification #8719

[extension] Use dedicated audience for dust api. Add scopes verification #8719

tdraier commented Nov 18, 2024 •

edited

Loading

github-actions bot commented Nov 18, 2024 •

edited

Loading

flvndvd left a comment

flvndvd Nov 19, 2024

Fraggle Nov 19, 2024 •

edited

Loading

Fraggle Nov 19, 2024

tdraier Nov 19, 2024

tdraier Nov 19, 2024

Fraggle commented Nov 19, 2024 •

edited

Loading

Fraggle commented Nov 19, 2024 •

edited

Loading

Fraggle commented Nov 19, 2024

Fraggle left a comment

Fraggle Nov 19, 2024

tdraier Nov 19, 2024

Fraggle Nov 19, 2024

tdraier Nov 19, 2024

Fraggle Nov 19, 2024

tdraier Nov 19, 2024

Fraggle Nov 19, 2024

tdraier Nov 19, 2024

Fraggle Nov 19, 2024

tdraier commented Nov 19, 2024

Fraggle commented Nov 19, 2024

Fraggle left a comment

[extension] Use dedicated audience for dust api. Add scopes verification #8719

[extension] Use dedicated audience for dust api. Add scopes verification #8719

Conversation

tdraier commented Nov 18, 2024 • edited Loading

Description

Risk

Deploy Plan

github-actions bot commented Nov 18, 2024 • edited Loading

flvndvd left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Fraggle Nov 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Fraggle commented Nov 19, 2024 • edited Loading

Fraggle commented Nov 19, 2024 • edited Loading

Fraggle commented Nov 19, 2024

Fraggle left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tdraier commented Nov 19, 2024

Fraggle commented Nov 19, 2024

Fraggle left a comment

Choose a reason for hiding this comment

tdraier commented Nov 18, 2024 •

edited

Loading

github-actions bot commented Nov 18, 2024 •

edited

Loading

Fraggle Nov 19, 2024 •

edited

Loading

Fraggle commented Nov 19, 2024 •

edited

Loading

Fraggle commented Nov 19, 2024 •

edited

Loading