feat: add mfa verification postgres hook #1314

J0 · 2023-11-20T16:34:07Z

What kind of change does this PR introduce?

Proof of concept hook for MFA Verification. With this hook, developers can introduce additional conditions around when to accept/reject an MFA verification (e.g. log a developer out after a certain number of attempts).

We distinguish this from the existing Webhooks implementation via introduction of hooks package which will contain future Hook related structs, constants, and utility methods.

For the most part we leverage existing Postgres capabilities - as far as possible we will return the PostgreSQL error codes for debugging and use Postgres in-built timeouts to ensure hte hook doesn't overrun.

Testing

The MFA Verification Hook test suite does not guarantee accurate status codes - the test setup (to enroll factors and create a challenge after signup) requires some setup. It is reliant on signUpAndVerify which gets the dev to AAL2 and takes time to refactor.

As such, most of the cases were manually tested in addition to the current loose check of checking for the absence of an access token. Further edits will be made in GMT +8 morning to properly check for the http status codes in the tests.

Also, since supabase_auth_admin cannot create functions on the public schema we create the functions on the auth schema for testing. We typically discourage this on the Supabase platform but in theory there should be no issue when dealing with GoTrue (the OSS project). Will spend a short amount of time looking into alternatives tomorrow.

Additional Notes

Response schema checks are left out of this PR as they don't seem to serve as much benefit for this particular extensibility point and will probably bloat the PR a little with the introduction of a new library

internal/api/auth_hooks.go

internal/api/mfa.go

internal/conf/configuration.go

internal/conf/configuration_test.go

internal/api/auth_hooks.go

…e/gotrue into j0/mfa_verification_counter_hook

## What kind of change does this PR introduce? The mfa tests are hard to read. There's also a lot of redundant code which makes testing for hooks quite a bit harder. This PR aims to remove some of the redundancy so that it's easier to write the tests for #1314 Main changes include - Splitting out `enrollAndVerify` into `enroll` and `verify` - Using suite specific constants - Packaging duplicated setup code --------- Co-authored-by: joel@joellee.org <joel@joellee.org>

…ication_counter_hook

internal/api/mfa.go

internal/conf/configuration.go

internal/api/mfa.go

internal/hooks/auth_hooks.go

Co-authored-by: Stojan Dimitrovski <sdimitrovski@gmail.com>

…e/gotrue into j0/mfa_verification_counter_hook

internal/api/mfa.go

J0 · 2023-11-30T09:44:00Z

internal/hooks/auth_hooks.go

+
+}
+
+func (mf *MFAVerificationAttemptOutput) IsError() bool {


WIll refactor this in the FLUP with password verification hooks

github-actions · 2023-11-30T09:48:13Z

🎉 This PR is included in version 2.123.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

## What kind of change does this PR introduce? The mfa tests are hard to read. There's also a lot of redundant code which makes testing for hooks quite a bit harder. This PR aims to remove some of the redundancy so that it's easier to write the tests for supabase#1314 Main changes include - Splitting out `enrollAndVerify` into `enroll` and `verify` - Using suite specific constants - Packaging duplicated setup code --------- Co-authored-by: joel@joellee.org <joel@joellee.org>

## What kind of change does this PR introduce? Proof of concept hook for MFA Verification. With this hook, developers can introduce additional conditions around when to accept/reject an MFA verification (e.g. log a developer out after a certain number of attempts). We distinguish this from the existing Webhooks implementation via introduction of `hooks` package which will contain future Hook related structs, constants, and utility methods. For the most part we leverage existing Postgres capabilities - as far as possible we will return the PostgreSQL error codes for debugging and use Postgres in-built timeouts to ensure hte hook doesn't overrun. ## Testing The MFA Verification Hook test suite does not guarantee accurate status codes - the test setup (to enroll factors and create a challenge after signup) requires some setup. It is reliant on `signUpAndVerify` which gets the dev to AAL2 and takes time to refactor. As such, most of the cases were manually tested in addition to the current loose check of checking for the absence of an access token. Further edits will be made in GMT +8 morning to properly check for the http status codes in the tests. Also, since `supabase_auth_admin` cannot create functions on the `public` schema we create the functions on the `auth` schema for testing. We typically discourage this on the Supabase platform but in theory there should be no issue when dealing with GoTrue (the OSS project). Will spend a short amount of time looking into alternatives tomorrow. ## Additional Notes Response schema checks are left out of this PR as they don't seem to serve as much benefit for this particular extensibility point and will probably bloat the PR a little with the introduction of a new library --------- Co-authored-by: joel@joellee.org <joel@joellee.org> Co-authored-by: Stojan Dimitrovski <sdimitrovski@gmail.com>

## What kind of change does this PR introduce? The mfa tests are hard to read. There's also a lot of redundant code which makes testing for hooks quite a bit harder. This PR aims to remove some of the redundancy so that it's easier to write the tests for supabase#1314 Main changes include - Splitting out `enrollAndVerify` into `enroll` and `verify` - Using suite specific constants - Packaging duplicated setup code --------- Co-authored-by: joel@joellee.org <joel@joellee.org>

## What kind of change does this PR introduce? Proof of concept hook for MFA Verification. With this hook, developers can introduce additional conditions around when to accept/reject an MFA verification (e.g. log a developer out after a certain number of attempts). We distinguish this from the existing Webhooks implementation via introduction of `hooks` package which will contain future Hook related structs, constants, and utility methods. For the most part we leverage existing Postgres capabilities - as far as possible we will return the PostgreSQL error codes for debugging and use Postgres in-built timeouts to ensure hte hook doesn't overrun. ## Testing The MFA Verification Hook test suite does not guarantee accurate status codes - the test setup (to enroll factors and create a challenge after signup) requires some setup. It is reliant on `signUpAndVerify` which gets the dev to AAL2 and takes time to refactor. As such, most of the cases were manually tested in addition to the current loose check of checking for the absence of an access token. Further edits will be made in GMT +8 morning to properly check for the http status codes in the tests. Also, since `supabase_auth_admin` cannot create functions on the `public` schema we create the functions on the `auth` schema for testing. We typically discourage this on the Supabase platform but in theory there should be no issue when dealing with GoTrue (the OSS project). Will spend a short amount of time looking into alternatives tomorrow. ## Additional Notes Response schema checks are left out of this PR as they don't seem to serve as much benefit for this particular extensibility point and will probably bloat the PR a little with the introduction of a new library --------- Co-authored-by: joel@joellee.org <joel@joellee.org> Co-authored-by: Stojan Dimitrovski <sdimitrovski@gmail.com>

## What kind of change does this PR introduce? The mfa tests are hard to read. There's also a lot of redundant code which makes testing for hooks quite a bit harder. This PR aims to remove some of the redundancy so that it's easier to write the tests for supabase#1314 Main changes include - Splitting out `enrollAndVerify` into `enroll` and `verify` - Using suite specific constants - Packaging duplicated setup code --------- Co-authored-by: joel@joellee.org <joel@joellee.org>

## What kind of change does this PR introduce? Proof of concept hook for MFA Verification. With this hook, developers can introduce additional conditions around when to accept/reject an MFA verification (e.g. log a developer out after a certain number of attempts). We distinguish this from the existing Webhooks implementation via introduction of `hooks` package which will contain future Hook related structs, constants, and utility methods. For the most part we leverage existing Postgres capabilities - as far as possible we will return the PostgreSQL error codes for debugging and use Postgres in-built timeouts to ensure hte hook doesn't overrun. ## Testing The MFA Verification Hook test suite does not guarantee accurate status codes - the test setup (to enroll factors and create a challenge after signup) requires some setup. It is reliant on `signUpAndVerify` which gets the dev to AAL2 and takes time to refactor. As such, most of the cases were manually tested in addition to the current loose check of checking for the absence of an access token. Further edits will be made in GMT +8 morning to properly check for the http status codes in the tests. Also, since `supabase_auth_admin` cannot create functions on the `public` schema we create the functions on the `auth` schema for testing. We typically discourage this on the Supabase platform but in theory there should be no issue when dealing with GoTrue (the OSS project). Will spend a short amount of time looking into alternatives tomorrow. ## Additional Notes Response schema checks are left out of this PR as they don't seem to serve as much benefit for this particular extensibility point and will probably bloat the PR a little with the introduction of a new library --------- Co-authored-by: joel@joellee.org <joel@joellee.org> Co-authored-by: Stojan Dimitrovski <sdimitrovski@gmail.com>

joel@joellee.org added 5 commits November 20, 2023 21:18

feat: initial commit

67ecb95

fix: add scaffolding

6078578

feat: generate payload

51e1987

feat: set up calling mechanism

1c15e18

feat: add more surrounding logic

c62ff8a

hf reviewed Nov 20, 2023

View reviewed changes

joel@joellee.org added 4 commits November 21, 2023 11:45

fix: reinstate relevant constants

fa1122a

feat: add minor validation and cleanup

da86cde

feat: add hook configuration to settings

185b1d7

feat: update naming conventions

6676552

hf reviewed Nov 24, 2023

View reviewed changes

internal/api/auth_hooks.go Outdated Show resolved Hide resolved

internal/api/auth_hooks.go Outdated Show resolved Hide resolved

internal/api/auth_hooks.go Outdated Show resolved Hide resolved

internal/api/auth_hooks.go Outdated Show resolved Hide resolved

joel@joellee.org and others added 15 commits November 25, 2023 00:30

feat:refactor to do w/o abstraction

9493543

fix: remove now redundant methods

4b08570

fix: add some logging

78e184c

fix: remove unused code

f3ae14b

Merge branch 'master' into j0/mfa_verification_counter_hook

8708df0

fix: reset unused files

dc9d6da

feat: add stubs, revert unneeded changes

1cd6eff

Merge branch 'j0/mfa_verification_counter_hook' of github.com:supabas…

3a93440

…e/gotrue into j0/mfa_verification_counter_hook

refactor: rename hook ep

8a3ec9c

refactor: rename HookErrorResponse->AuthHookErrorResponse

e21a806

test: add initial tests

b7a8c23

feat: properly fetch response

5a4ce40

feat: update tests

aa9d920

feat: add a few tests

176ad48

fix: update test structure

32b1a8d

J0 changed the title ~~[wip]: feat: add mfa verification postgres hook poc~~ feat: add mfa verification postgres hook poc Nov 27, 2023

joel@joellee.org added 3 commits November 27, 2023 18:59

feat: add local timeout and more tests

b9d6ba7

refactor: cut back on redundant code

b50b7ed

fix: patch tests

651151e

joel@joellee.org added 2 commits November 28, 2023 23:58

chore: convert fetchhookname into configuration load

ddae946

fix: light refactor of tests

25f95f4

joel@joellee.org added 4 commits November 29, 2023 13:47

Merge branch 'master' of github.com:supabase/gotrue into j0/mfa_verif…

7693417

…ication_counter_hook

fix: add status code check

af2c255

refactor: use errors

7b874b8

refactor: shove config back to configuration

447de5f

J0 marked this pull request as ready for review November 29, 2023 17:48

hf reviewed Nov 29, 2023

View reviewed changes

J0 and others added 5 commits November 30, 2023 09:04

Update internal/conf/configuration.go

18ccfc6

Co-authored-by: Stojan Dimitrovski <sdimitrovski@gmail.com>

Update internal/hooks/auth_hooks.go

be4fd32

Co-authored-by: Stojan Dimitrovski <sdimitrovski@gmail.com>

refactor: use error interface instead

a15631b

Merge branch 'j0/mfa_verification_counter_hook' of github.com:supabas…

8641c92

…e/gotrue into j0/mfa_verification_counter_hook

fix: make schema constant

2d512cb

hf reviewed Nov 30, 2023

View reviewed changes

internal/api/mfa.go Outdated Show resolved Hide resolved

hf changed the title ~~feat: add mfa verification postgres hook poc~~ feat: add mfa verification postgres hook Nov 30, 2023

joel@joellee.org added 2 commits November 30, 2023 17:17

test: reinstate test suite

3c99bde

fix: remove unused structs

3699c01

J0 merged commit db344d5 into master Nov 30, 2023
2 checks passed

J0 deleted the j0/mfa_verification_counter_hook branch November 30, 2023 09:43

J0 commented Nov 30, 2023

View reviewed changes

github-actions bot added the released label Nov 30, 2023

J0 mentioned this pull request Dec 4, 2023

Add password verification hook #1328

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add mfa verification postgres hook #1314

feat: add mfa verification postgres hook #1314

J0 commented Nov 20, 2023 •

edited

Loading

J0 Nov 30, 2023

github-actions bot commented Nov 30, 2023

feat: add mfa verification postgres hook #1314

feat: add mfa verification postgres hook #1314

Conversation

J0 commented Nov 20, 2023 • edited Loading

What kind of change does this PR introduce?

Testing

Additional Notes

J0 Nov 30, 2023

Choose a reason for hiding this comment

github-actions bot commented Nov 30, 2023

J0 commented Nov 20, 2023 •

edited

Loading