feat(users): setup user authentication methods schema and apis #4999

apoorvdixit88 · 2024-06-13T12:10:56Z

Type of Change

Description

The PR:

Sets up schema for user authentication methods
adds queries to interact with schema
adds endpoints to create, update and list authentication methods

Additional Changes

This PR modifies the API contract
This PR modifies the database schema
This PR modifies application configuration/environment variables

config/config.example.toml
config/deployments/env_specific.toml
config/development.toml
config/docker_compose.toml

Motivation and Context

Closes #4998

How did you test it?

To create authentication method, ( auth is admin_api_key only):

curl --location 'http://localhost:8080/user/auth' \
--header 'Content-Type: application/json' \
--header 'api-key: test_admin' \
--header 'Cookie: Cookie_1=value' \
--data '
{
"owner_id": "pw",
    "owner_type": "tenant",
    "auth_method": {
        "auth_type": "open_id_connect",
        "private_config": {
            "base_url": "hello.com",
            "client_id": "something",
            "client_secret": "something",
            "private_key": "something"
        },
        "public_config": {
            "name": "okta"
        }
    },
    "allow_signup": true
    }'

Response: 200 OK if auth method got created successfully.

To update

curl --location --request PUT 'http://localhost:8080/user/auth' \
--header 'Content-Type: application/json' \
--header 'api-key: test_admin' \
--header 'Cookie: Cookie_1=value' \
--data '{
    "id": "65ff02eb-f3dc-4afc-b8ac-cef2a30c4d12",
    "auth_method": {
        "auth_type": "open_id_connect",
        "private_config": {
            "base_url": "hello.com",
            "client_id": "something",
            "client_secret": "something_update",
            "private_key": "something_update"
        },
        "public_config": {
            "name": "okta"
        }
    }
}
'

Response will be 200 OK for success full auth config update

To get list of authentication methods

curl --location 'http://localhost:8080/user/auth/list?auth_id=cbd4e678-6c09-4211-8611-74700aa1f695' \
--header 'Cookie: Cookie_1=value' \
--data ''

Response

[
    {
        "id": "6b543648-4d47-498d-bcc2-68132dbf8521",
        "auth_id": "cbd4e678-6c09-4211-8611-74700aa1f695",
        "auth_method": {
            "type": "open_id_connect",
            "name": "okta"
        },
        "allow_signup": true
    },
    {
        "id": "1364c40d-1c5a-4dc9-ae28-ce908f537e83",
        "auth_id": "cbd4e678-6c09-4211-8611-74700aa1f695",
        "auth_method": {
            "type": "password",
            "name": null
        },
        "allow_signup": true
    }
]

Checklist

I formatted the code cargo +nightly fmt --all
I addressed lints thrown by cargo clippy
I reviewed the submitted code
I added unit tests for my changes where possible

racnan · 2024-06-14T09:22:25Z

auth_config should not be exposed.
Create a auth_config type in application.

ThisIsMani

Should we encrypt auth_configs in DB? It might contain merchant specific secrets.

migrations/2024-06-10-084722_create_org_authentication_table/up.sql

migrations/2024-06-10-084722_create_org_authentication_table/down.sql

crates/router/src/routes/user.rs

migrations/2024-06-10-084722_create_org_authentication_table/up.sql

crates/router/src/core/errors/user.rs

crates/router/src/db/org_authentication_method.rs

ThisIsMani

Are we sure that we are going to create separate table to tenants?

migrations/2024-06-10-084722_create_org_authentication_table/up.sql

crates/diesel_models/src/org_authentication_method.rs

crates/router/src/core/user.rs

ThisIsMani

Where are we going to store totp_required? Neither we have a column in the table nor AuthConfig have any field for this.

crates/api_models/src/user.rs

crates/router/src/core/user.rs

migrations/2024-06-10-084722_create_user_authentication_methods_table/down.sql

racnan · 2024-06-20T08:26:03Z

Where are we going to store totp_required? Neither we have a column in the table nor AuthConfig have any field for this.

@ThisIsMani for now we can assume totp_required: true for password and magic link flow and totp_required: false for sso. will add a column/config later when exceptions come.

…cker_compose.toml`

crates/api_models/src/user.rs

crates/diesel_models/src/query/user_authentication_method.rs

inventvenkat · 2024-06-20T13:51:05Z

crates/router/src/core/user.rs

+    state: SessionState,
+    req: user_api::GetUserAuthenticationMethodsRequest,
+) -> UserResponse<Vec<user_api::UserAuthenticationMethodResponse>> {
+    let user_auth_encryption_key = hex::decode(


We should not do KMS decode every time, it will increase the latency and the cost. We can instead cache the decoded data.

Let's reconsider encrypting the whole config payload : Do we really need to encrypt the whole config data? Because the response needs only non sensitive data from config. By eliminating encryption part in non sensitive data, we can skip this decryption part and also the KMS part in this API.

racnan · 2024-06-21T08:19:10Z

crates/api_models/src/user.rs

+pub struct AuthMethodDetails {
+    #[serde(rename = "type")]
+    pub auth_type: common_enums::UserAuthType,
+    pub name: Option<OpenIdProvider>,


This should not be forced to take OpenIdProvider type.

This can be changed to String when we will add other cases, where name doesn't come from OpenIdProvider

racnan · 2024-06-21T08:21:48Z

crates/common_enums/src/enums.rs

+pub enum Owner {
+    Organization,
+    Tenant,
+    Internal,


What would be the owner_id, in case on Owner::Internal.

we can remove Internal, or assign some id (hyperswitch) if we think we encounter relevant use cases for this.

racnan · 2024-06-21T08:59:41Z

crates/diesel_models/src/user_authentication_method.rs

+#[derive(router_derive::Setter, Clone, Debug, Insertable, router_derive::DebugAsDisplay)]
+#[diesel(table_name = user_authentication_methods)]
+pub struct UserAuthenticationMethodNew {
+    pub id: String,
+    pub auth_id: String,
+    pub owner_id: String,
+    pub owner_type: enums::Owner,
+    pub auth_type: enums::UserAuthType,
+    pub private_config: Option<Encryption>,
+    pub public_config: Option<serde_json::Value>,
+    pub allow_signup: bool,
+    pub created_at: PrimitiveDateTime,
+    pub last_modified_at: PrimitiveDateTime,
+}


If the this type is exactly same as UserAuthenticationMethod, then why is this struct required.
can't we put the required macros in UserAuthenticationMethod?

In our diesel models we have like this, we use Insertable for new type and use the other as response type. Better to keep this distinction between types and keep things consistent.

racnan · 2024-06-21T10:35:12Z

crates/router/src/core/user.rs

+    .change_context(UserErrors::InternalServerError)
+    .attach_printable("Failed to decode DEK")?;
+
+    let (private_config, public_config) = match req.auth_method {


repeated code.

we can cover this along with constraint addition in upcoming PR

…ough-hyperswitch-cypress * 'main' of github.com:juspay/hyperswitch: feat(router): add support for googlepay step up flow (#2744) fix(access_token): use `merchant_connector_id` in access token (#5106) feat: added kafka events for authentication create and update (#4991) feat(ci): add vector to handle logs pipeline (#5021) feat(users): Decision manager flow changes for SSO (#4995) ci(cypress): Fix payment method id for non supported connectors (#5075) refactor(core): introduce an interface to switch between old and new connector integration implementations on the connectors (#5013) refactor(events): populate object identifiers in outgoing webhooks analytics events during retries (#5067) Refactor: [Fiserv] Remove Default Case Handling (#4767) chore(version): 2024.06.24.0 fix(router): avoid considering pre-routing results during `perform_session_token_routing` (#5076) refactor(redis): spawn one subscriber thread for handling all the published messages to different channel (#5064) feat(users): setup user authentication methods schema and apis (#4999) feat(payment_methods): Implement Process tracker workflow for Payment method Status update (#4668) chore(version): 2024.06.20.1 chore(postman): update Postman collection files fix(payment_methods): support last used for off session token payments (#5039) ci(postman): add net_amount field test cases (#3286) refactor(connector): [Mifinity]dynamic fields for mifinity (#5056) refactor(payment_method): [Klarna] store and populate payment_type for klarna_sdk Paylater in response (#4956)

apoorvdixit88 added 5 commits June 12, 2024 16:39

feat(users): setup table for org authentication methods

397368a

feat(users): add api to get org auth methods

a21f2e9

feat(users): implement core functions for create and update

556f416

fix: change error for create and update

b58bfab

fix: remove warnings

8b4c7bc

apoorvdixit88 requested review from inventvenkat, racnan and ThisIsMani June 13, 2024 12:10

apoorvdixit88 self-assigned this Jun 13, 2024

apoorvdixit88 requested review from a team as code owners June 13, 2024 12:10

hyperswitch-bot bot removed the M-api-contract-changes Metadata: This PR involves API contract changes label Jun 13, 2024

apoorvdixit88 and others added 2 commits June 13, 2024 17:43

fix: line formatting

92dfc93

chore: run formatter

015d7e7

ThisIsMani previously approved these changes Jun 14, 2024

View reviewed changes

ThisIsMani reviewed Jun 14, 2024

View reviewed changes

fix: update schema for db

3dc3295

apoorvdixit88 dismissed ThisIsMani’s stale review via 3dc3295 June 14, 2024 12:54

fix: update config request

aa5fbc0

ThisIsMani reviewed Jun 19, 2024

View reviewed changes

update core function

ae64997

apoorvdixit88 force-pushed the setup-org-authentication-methods-table branch from b6a1adc to ae64997 Compare June 19, 2024 13:50

apoorvdixit88 added 2 commits June 19, 2024 19:56

fix: rename errors and interface

bfa2a47

fix: serde to snake case for auth config

4c40b6c

ThisIsMani reviewed Jun 20, 2024

View reviewed changes

crates/api_models/src/user.rs Outdated Show resolved Hide resolved

crates/router/src/core/user.rs Outdated Show resolved Hide resolved

migrations/2024-06-10-084722_create_user_authentication_methods_table/down.sql Outdated Show resolved Hide resolved

resolve review comments

60905a6

ThisIsMani added the M-configuration-changes Metadata: This PR involves configuration changes label Jun 20, 2024

ThisIsMani and others added 4 commits June 20, 2024 14:56

refactor: add name in list api response

e4d66c5

add enum for name

bf4ef76

refactor: remove key from list api

257f816

refactor: remove comments

91cd8d2

apoorvdixit88 changed the title ~~feat(users): setup org authentication methods schema and apis~~ feat(users): setup authentication methods schema and apis Jun 20, 2024

apoorvdixit88 changed the title ~~feat(users): setup authentication methods schema and apis~~ feat(users): setup user authentication methods schema and apis Jun 20, 2024

apoorvdixit88 assigned ThisIsMani Jun 20, 2024

refactor: add sample keys in config/development.toml and `config/do…

bbd1c0b

…cker_compose.toml`

ThisIsMani previously approved these changes Jun 20, 2024

View reviewed changes

SanchithHegde previously approved these changes Jun 20, 2024

View reviewed changes

inventvenkat requested changes Jun 20, 2024

View reviewed changes

refactor: separate out public and private configs

e63a327

ThisIsMani dismissed stale reviews from SanchithHegde and themself via e63a327 June 20, 2024 16:40

ThisIsMani requested a review from inventvenkat June 21, 2024 06:21

apoorvdixit88 requested review from ThisIsMani and removed request for ThisIsMani June 21, 2024 07:38

inventvenkat approved these changes Jun 21, 2024

View reviewed changes

racnan approved these changes Jun 21, 2024

View reviewed changes

likhinbopanna added this pull request to the merge queue Jun 21, 2024

Merged via the queue into main with commit 2005d3d Jun 21, 2024
31 checks passed

likhinbopanna deleted the setup-org-authentication-methods-table branch June 21, 2024 11:14

SanchithHegde removed the S-waiting-on-review Status: This PR has been implemented and needs to be reviewed label Jun 23, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(users): setup user authentication methods schema and apis #4999

feat(users): setup user authentication methods schema and apis #4999

apoorvdixit88 commented Jun 13, 2024 •

edited

Loading

racnan commented Jun 14, 2024

ThisIsMani left a comment

ThisIsMani left a comment

ThisIsMani left a comment •

edited

Loading

racnan commented Jun 20, 2024

inventvenkat Jun 20, 2024

racnan Jun 21, 2024

apoorvdixit88 Jun 21, 2024

racnan Jun 21, 2024

apoorvdixit88 Jun 21, 2024 •

edited

Loading

racnan Jun 21, 2024

apoorvdixit88 Jun 21, 2024

racnan Jun 21, 2024

apoorvdixit88 Jun 21, 2024

feat(users): setup user authentication methods schema and apis #4999

feat(users): setup user authentication methods schema and apis #4999

Conversation

apoorvdixit88 commented Jun 13, 2024 • edited Loading

Type of Change

Description

Additional Changes

Motivation and Context

How did you test it?

Checklist

racnan commented Jun 14, 2024

ThisIsMani left a comment

Choose a reason for hiding this comment

ThisIsMani left a comment

Choose a reason for hiding this comment

ThisIsMani left a comment • edited Loading

Choose a reason for hiding this comment

racnan commented Jun 20, 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

apoorvdixit88 Jun 21, 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

Choose a reason for hiding this comment

apoorvdixit88 commented Jun 13, 2024 •

edited

Loading

ThisIsMani left a comment •

edited

Loading

apoorvdixit88 Jun 21, 2024 •

edited

Loading