Feat: Configure refresh token expiry #4525

thejwuscript · 2024-05-07T15:36:24Z

Related to issue #4409

This PR allows project admins to optionally configure the expiry of a refresh token for a ClientApplication.

A refreshTokenExpiry field is added to the ClientApplication resource. The field is optional to allow backward compatibility. The value is used to generate the exp claim on a refresh token.

UI changes to the Medplum App is not addressed in this PR; it can be included here if necessary, or be made on a separate PR.

vercel · 2024-05-07T15:36:29Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
medplum-provider	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jun 11, 2024 7:32am

3 Ignored Deployments

Name	Status	Preview	Updated (UTC)
medplum-app	⬜️ Ignored (Inspect)	Visit Preview	Jun 11, 2024 7:32am
medplum-storybook	⬜️ Ignored (Inspect)	Visit Preview	Jun 11, 2024 7:32am
medplum-www	⬜️ Ignored (Inspect)	Visit Preview	Jun 11, 2024 7:32am

vercel · 2024-05-07T15:36:31Z

@thejwuscript is attempting to deploy a commit to the Medplum Team on Vercel.

A member of the Team first needs to authorize it.

josh- · 2024-05-29T06:13:33Z

packages/definitions/dist/fhir/r4/profiles-medplum.json

+ "min" : 0,
+ "max" : "1",
+ "type" : [{
+ "code" : "date"


for our use case we'd like to specify a value here which is relative to the current time, not a fixed date parameter.

the expiration value is passed to the jose npm package, which treats number and Date values as the fixed expiration time in the claim (reference here), however string values are converted to a time span relative to the current time (reference here) - which matches the existing behaviour with the 2w default.

i think ideally this property would be a string which could either support the full range of jose formats, or perhaps a subset of this (to allow for easier validation in the medplum middleware)

Yes I think if we can change the name of this field to refreshTokenLifetime and make it an expiry duration as a string (eg. '2w', '1h', etc.) that maps to jose formats as @josh- pointed out, that would be ideal 👍

@thejwuscript - just wanted to make sure you saw the above comment from @ThatOneBro . This is our main feedback on the PR

Yes, thanks very much for the feedback. I'll be making the changes.

@thejwuscript do you know when you'll be able to make those changes? let me know if I can help. eager to be able to use this.

@josh- I'll try get it done by Monday.

ThatOneBro

Looks good for the most part! Consulting with @mattwiller about where the validation should occur. Once we figure that out we should be good to go! Thanks again for your help @thejwuscript! 😄

ThatOneBro · 2024-06-10T21:15:30Z

packages/server/src/fhir/routes.ts

@@ -117,6 +118,9 @@ protectedRoutes.use('/bulkdata', bulkDataRouter);
 // Async Job
 protectedRoutes.use('/job', jobRouter);

+// Validate data before updating ClientApplication
+protectedRoutes.put('/ClientApplication/:id', saveClientValidator);


I'm thinking this probably should not be directly in an express route, should probably be done at the FhirRouter level potentially? What do you think, @mattwiller?

I know we have talked about doing something like this across the board long term (having specific validation on a per resource basis, maybe via some "prewrite" pass)

That's definitely out of scope for this PR but would be nice to move in a more uniform direction

I would actually propose moving this validation into the field definition itself, using a FHIRPath condition like this:

{ "id" : "ClientApplication.refreshTokenLifetime", "path" : "ClientApplication.refreshTokenLifetime", "definition" : "Optional configuration to set the refresh token duration", "min" : 0, "max" : "1", "type" : [{ "code" : "string" }], "constraint" : [{ "key" : "clapp-1", "severity" : "error", "human" : "Token lifetime must be a valid string representing time duration (eg. 2w, 1h)", "expression" : "$this.matches('^[0-9]+[smhdwy]$')" }], "base": { "path" : "ClientApplication.refreshTokenLifetime", "min" : 0, "max" : "1" } }

That way, this validation gets performed at the Repository level, any time the resource is updated anywhere in the system.

Yeah I actually agree, this is probably the right way to do it

ThatOneBro · 2024-06-11T02:00:17Z

Okay I pushed a tweak applying @mattwiller's suggestion and some tests for some specific cases, after someone else approves we can push this through. Thank you @thejwuscript for taking this issue 🙏

packages/server/src/admin/client.ts

packages/server/src/oauth/keys.ts

## What's Changed fix(ci): fix `\n` missing due to reversal, use `git log --reverse` (#4649) Allow chained search in _filter (#4647) Polish eligibility demo (#4637) cleanup(core): remove stray `console.log` (#4653) Update sidebar.ts (#4652) Feat: Configure refresh token expiry (#4525) Dependency upgrades 2024-06-10 (#4650) cleanup(chart-demo): rm ignored `example-bots.json` (#4656) Adding instructions to the example app READMEs on how to build bots (#4660) Update useSearch.ts (#4663) Export QuestionnaireFormContext and QuestionnairePageSequence from QuestionnaireForm (#4664) Add `expo-polyfills` to README packages list (#4666) revert(react): remove export of `QuestionnairePageSequence` (#4669) Fix wrong pipe character in README (#4671) Document Terminology Service operation endpoints (#4665) Dependency upgrades 2024-06-17 (#4673) Gracefully handle no major dep upgrades (#4675) SQL on FHIR ViewDefinition types (#4674) fix(build) Update deprecated import assertion into import attribute (#4682) Fixes #4398 - add mapByIdentifier util function (#4635) Fixes #4600 - Add Auto Confirmation Parameter for Headless Deployment (#4625) fix(react-hooks): make `loading` track `MedplumClient#isLoading()` (#4677) Implements FHIRPath string join (stu) (#4683) SQL-on-FHIR processResource (#4678) feat(useSubscription): add `subscriptionProps` as optional param (#4180) Resolve conditional references (#4633) feat(subscriptions): add `unbind-from-token` message for WebSocket subscriptions (#4672) Document remaining Terminology Service operations (#4680) feat(agent): add `keepAlive` setting to `Agent` (#4657) Update README.md (#4687) Fix all copyright dates (#4689) Fixes subject input on PlanDefinitionApplyForm (#4699) Deprecate non-strict mode (#4651) Validate certain references with systemRepo (#4700) docs(useSubscription): clean up examples, add JSDoc comment (#4692) cleanup(repo): `handleMaybeCacheOnly` -> `handleStorage` (#4696) Minor fixes to Eligibility Demo (#4703) **Full Changelog**: v3.1.8...v3.1.9

thejwuscript requested a review from a team as a code owner May 7, 2024 15:36

vercel bot deployed to Preview – medplum-provider May 7, 2024 15:41 View deployment

reshmakh added this to the May 31st, 2024 milestone May 10, 2024

vercel bot deployed to Preview – medplum-provider May 17, 2024 18:04 View deployment

josh- reviewed May 29, 2024

View reviewed changes

reshmakh modified the milestones: May 31st, 2024, June 30, 2024 Jun 2, 2024

vercel bot deployed to Preview – medplum-provider June 8, 2024 16:38 View deployment

thejwuscript requested a review from ThatOneBro June 8, 2024 16:44

ThatOneBro reviewed Jun 10, 2024

View reviewed changes

thejwuscript and others added 7 commits June 10, 2024 16:13

Add new definitions

6fd0c56

Add validation when creating client

3e99e48

Pass date arg around to include in refresh token

e6c1382

Generate correct exp claim

c1f0081

Fix syntax

d5a82f8

Change field name and improve validation

3023d29

tweak(oauth): validate refreshTokenLifetime at resource level

65b5f32

ThatOneBro force-pushed the config-refresh-token branch from a22c9f7 to 65b5f32 Compare June 11, 2024 01:12

vercel bot deployed to Preview – medplum-provider June 11, 2024 01:13 View deployment

rahul1 reviewed Jun 11, 2024

View reviewed changes

packages/server/src/admin/client.ts Outdated Show resolved Hide resolved

rahul1 reviewed Jun 11, 2024

View reviewed changes

packages/server/src/oauth/keys.ts Outdated Show resolved Hide resolved

tweak(token): PR feedback

76dfe97

vercel bot deployed to Preview – medplum-provider June 11, 2024 07:32 View deployment

rahul1 approved these changes Jun 11, 2024

View reviewed changes

ThatOneBro added this pull request to the merge queue Jun 11, 2024

ThatOneBro self-assigned this Jun 11, 2024

ThatOneBro added the auth Authentication and authorization features and fixes label Jun 11, 2024

Merged via the queue into medplum:main with commit 35e34fd Jun 11, 2024
28 of 31 checks passed

ThatOneBro mentioned this pull request Jun 11, 2024

Allow Project Admins to configure refresh token expiry #4409

Closed

medplumbot mentioned this pull request Jun 21, 2024

Release Version 3.1.9 #4704

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat: Configure refresh token expiry #4525

Feat: Configure refresh token expiry #4525

thejwuscript commented May 7, 2024

vercel bot commented May 7, 2024 •

edited

Loading

vercel bot commented May 7, 2024

josh- May 29, 2024

ThatOneBro May 29, 2024

rahul1 May 30, 2024

thejwuscript May 30, 2024

josh- Jun 5, 2024

thejwuscript Jun 6, 2024

ThatOneBro left a comment

ThatOneBro Jun 10, 2024 •

edited

Loading

mattwiller Jun 10, 2024

ThatOneBro Jun 10, 2024

ThatOneBro commented Jun 11, 2024

Feat: Configure refresh token expiry #4525

Feat: Configure refresh token expiry #4525

Conversation

thejwuscript commented May 7, 2024

vercel bot commented May 7, 2024 • edited Loading

vercel bot commented May 7, 2024

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

ThatOneBro left a comment

Choose a reason for hiding this comment

ThatOneBro Jun 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ThatOneBro commented Jun 11, 2024

vercel bot commented May 7, 2024 •

edited

Loading

ThatOneBro Jun 10, 2024 •

edited

Loading