Authorise email retrieval relative to JWT authentication #1460

iamacook · 2024-04-25T11:19:22Z

Summary

Prior to this, retrieving accounts required a user to sign a message with every request. This migrates the suboptimal authentication/authorisation to the new JWT/SIWE-based approach.

Changes

Replace EmailRetrievalGuard and TimestampGuard guards with AuthGuard
Delete EmailRetrievalGuard
Checksum signer of request
Ensure authorisation payload exists and the chain_id and signer_address matches that of the request
Add/update test coverage accordingly.

iamacook · 2024-04-25T11:20:07Z

src/domain/account/account.repository.interface.ts

@@ -6,7 +6,7 @@ export interface IAccountRepository {
  getAccount(args: {
    chainId: string;
    safeAddress: string;
-    signer: string;
+    signer: `0x${string}`;


These type changes and addition of a AddressSchema validation of the :signer ensures that the address is checksummed for comparison against the JWT token.

iamacook · 2024-04-25T11:20:32Z

src/routes/email/email.controller.ts

  @UseFilters(AccountDoesNotExistExceptionFilter)
  async getEmail(
    @Param('chainId') chainId: string,
    @Param('safeAddress') safeAddress: string,
-    @Param('signer') signer: string,
+    @Param('signer', new ValidationPipe(AddressSchema)) signer: `0x${string}`,


This is where the address is checksummed.

coveralls · 2024-04-25T12:19:31Z

Pull Request Test Coverage Report for Build 8965479414

Details

10 of 10 (100.0%) changed or added relevant lines in 4 files are covered.
1 unchanged line in 1 file lost coverage.
Overall coverage decreased (-0.06%) to 92.166%

Files with Coverage Reduction	New Missed Lines	%
src/routes/transactions/entities/tests/human-description.builder.ts	1	90.0%

Totals
Change from base Build 8923924899:	-0.06%
Covered Lines:	6892
Relevant Lines:	7208

💛 - Coveralls

hectorgomezv

👏🏻

src/domain/account/account.repository.ts

src/domain/auth/auth.repository.interface.ts

iamacook · 2024-04-29T14:16:32Z

~~I'm going to mark this as a draft and abstract the AuthPayload entity into a separate PR to reduce the scope of this.~~

iamacook · 2024-04-29T16:12:33Z

src/routes/recovery/recovery.controller.spec.ts

@@ -58,6 +63,8 @@ describe('Recovery (Unit)', () => {
    const moduleFixture: TestingModule = await Test.createTestingModule({
      imports: [AppModule.register(testConfiguration)],
    })
+      .overrideModule(JWT_CONFIGURATION_MODULE)


As this opts in to features.email, the override is needed asAuthGuard is now on the EmailController.

iamacook · 2024-04-29T16:12:38Z

src/routes/subscriptions/subscription.controller.spec.ts

@@ -49,6 +54,8 @@ describe('Subscription Controller tests', () => {
    const moduleFixture: TestingModule = await Test.createTestingModule({
      imports: [AppModule.register(testConfiguration), EmailControllerModule],
    })
+      .overrideModule(JWT_CONFIGURATION_MODULE)


As this opts in to features.email, the override is needed asAuthGuard is now on the EmailController.

iamacook · 2024-04-29T16:16:11Z

src/routes/alerts/alerts.controller.spec.ts

@@ -112,6 +117,8 @@ describe('Alerts (Unit)', () => {
      const moduleFixture: TestingModule = await Test.createTestingModule({
        imports: [AppModule.register(testConfiguration)],
      })
+        .overrideModule(JWT_CONFIGURATION_MODULE)


As this opts in to features.email, the override is needed asAuthGuard is now on the EmailController.

fmrsabino

👏 🚀

Rename JwtRepository to AuthRepository

53f9ea7

iamacook self-assigned this Apr 25, 2024

iamacook requested a review from a team as a code owner April 25, 2024 11:19

iamacook commented Apr 25, 2024

View reviewed changes

iamacook added 2 commits April 25, 2024 14:29

Move repository back to domain

07bb4a1

Populate Request['auth'] in AuthGuard

770f9c7

iamacook force-pushed the auth-decorator branch from e34243f to 770f9c7 Compare April 25, 2024 12:34

iamacook force-pushed the get-email-jwt branch from f2a64fa to 206748d Compare April 25, 2024 12:34

Fix entity import

658dd45

iamacook force-pushed the get-email-jwt branch from fe5d208 to 6a25e3e Compare April 25, 2024 12:50

iamacook added 2 commits April 25, 2024 15:47

Merge branch 'main' into auth-decorator

0e6cbde

Rename AuthPayloadDecorator to Auth

3d66d93

iamacook force-pushed the get-email-jwt branch from c9d97e0 to 0a6edd7 Compare April 26, 2024 09:22

hectorgomezv approved these changes Apr 26, 2024

View reviewed changes

src/domain/account/account.repository.ts Outdated Show resolved Hide resolved

iamacook requested a review from hectorgomezv April 26, 2024 10:31

Add decorator test and rename request property

f047089

hectorgomezv previously approved these changes Apr 26, 2024

View reviewed changes

iamacook added 11 commits April 26, 2024 14:42

Authorise email retrieval relative to JWT authentication

3c3646d

Enable feature flag in tests

57060c8

Use test JWT configuration

358f767

Fix entity import

aff0c0b

Fix import

da0f933

Check datasource isn't called

ddac3d2

Newlines

11fc3e4

Assign signer_address as Safe owner

87746a5

Revert owner set

c2e8478

Refactor variable

4e90cee

Refactor variable

3adbf4b

fmrsabino removed the in review Someone is reviewing this Pull Request label Apr 29, 2024

Address comments

b47fd9c

iamacook requested a review from fmrsabino April 29, 2024 11:14

fmrsabino reviewed Apr 29, 2024

View reviewed changes

src/domain/auth/auth.repository.interface.ts Outdated Show resolved Hide resolved

Convert AuthPayload to class with helpers

f3a0bee

iamacook requested a review from fmrsabino April 29, 2024 12:49

Fix types

d2e2805

hectorgomezv approved these changes Apr 29, 2024

View reviewed changes

iamacook added 2 commits April 29, 2024 15:14

Don't return instance when working with token

817ad78

Only instantiate in decorator

e6870b7

iamacook requested a review from hectorgomezv April 29, 2024 13:21

iamacook added 2 commits April 29, 2024 15:45

Remove import

ad35b66

Don't validate + rename vars

d005133

iamacook marked this pull request as draft April 29, 2024 14:16

iamacook added 2 commits April 29, 2024 18:08

Merge branch 'main' into get-email-jwt

1420fde

Revert variable rename and opt out of auth

b4adbf0

iamacook commented Apr 29, 2024

View reviewed changes

Fix tests

4ad2d38

iamacook commented Apr 29, 2024

View reviewed changes

iamacook marked this pull request as ready for review April 29, 2024 16:22

fmrsabino added the in review Someone is reviewing this Pull Request label Apr 30, 2024

fmrsabino approved these changes Apr 30, 2024

View reviewed changes

fmrsabino removed the in review Someone is reviewing this Pull Request label Apr 30, 2024

Merge branch 'main' into get-email-jwt

a2c5e1a

iamacook requested a review from fmrsabino May 6, 2024 07:19

fmrsabino approved these changes May 6, 2024

View reviewed changes

iamacook merged commit 8a4e421 into main May 6, 2024
16 checks passed

iamacook deleted the get-email-jwt branch May 6, 2024 10:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Authorise email retrieval relative to JWT authentication #1460

Authorise email retrieval relative to JWT authentication #1460

iamacook commented Apr 25, 2024 •

edited

iamacook Apr 25, 2024

iamacook Apr 25, 2024

coveralls commented Apr 25, 2024 •

edited

hectorgomezv left a comment

iamacook commented Apr 29, 2024 •

edited

iamacook Apr 29, 2024

iamacook Apr 29, 2024

iamacook Apr 29, 2024

fmrsabino left a comment

Authorise email retrieval relative to JWT authentication #1460

Authorise email retrieval relative to JWT authentication #1460

Conversation

iamacook commented Apr 25, 2024 • edited

Summary

Changes

iamacook Apr 25, 2024

Choose a reason for hiding this comment

iamacook Apr 25, 2024

Choose a reason for hiding this comment

coveralls commented Apr 25, 2024 • edited

Pull Request Test Coverage Report for Build 8965479414

Details

💛 - Coveralls

hectorgomezv left a comment

Choose a reason for hiding this comment

iamacook commented Apr 29, 2024 • edited

iamacook Apr 29, 2024

Choose a reason for hiding this comment

iamacook Apr 29, 2024

Choose a reason for hiding this comment

iamacook Apr 29, 2024

Choose a reason for hiding this comment

fmrsabino left a comment

Choose a reason for hiding this comment

iamacook commented Apr 25, 2024 •

edited

coveralls commented Apr 25, 2024 •

edited

iamacook commented Apr 29, 2024 •

edited