Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(users): Create generate recovery codes API #4708

Merged
merged 3 commits into from
May 22, 2024

Conversation

ThisIsMani
Copy link
Contributor

@ThisIsMani ThisIsMani commented May 20, 2024

Type of Change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring
  • Dependency updates
  • Documentation
  • CI/CD

Description

Currently recovery code are being generated in begin_totp API. We want to make a separate out recovery codes and TOTP, so this PR creates a new API which generates the recovery codes.

This API will also be used to regenerate recovery codes later.

Additional Changes

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

Motivation and Context

This is a sub part of #4707

How did you test it?

This can only be tested locally as it requires some changes in the redis.

curl --location 'http://localhost:8080/user/recovery_codes/generate' \
--header 'Authorization: Bearer SPT with purpose as TOTP' \
{
    "recovery_codes": [
        "w5fM-NLm0",
        "TumH-oyXE",
        "wDIr-zEmu",
        "HZjm-e16M",
        "Q4qB-MupL",
        "5BtN-vRuY",
        "4vG6-URGf",
        "Dbq8-n5V1"
    ]
}

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

@ThisIsMani ThisIsMani added C-feature Category: Feature request or enhancement S-waiting-on-review Status: This PR has been implemented and needs to be reviewed A-users Area: Users labels May 20, 2024
@ThisIsMani ThisIsMani self-assigned this May 20, 2024
@ThisIsMani ThisIsMani requested review from a team as code owners May 20, 2024 13:58
@@ -68,8 +68,10 @@ pub enum UserErrors {
RoleNameAlreadyExists,
#[error("TOTPNotSetup")]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also should be Totp considering below two

@@ -1631,7 +1631,7 @@ pub async fn begin_totp(
}));
}

let totp = utils::user::generate_default_totp(user_from_db.get_email(), None)?;
let totp = utils::user::two_fa::generate_default_totp(user_from_db.get_email(), None)?;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be better if we call this function two_factor_authentication?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is a module and I feel it will be too big for a file name.
Opinions @racnan

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

two_factor_auth ?

@ThisIsMani ThisIsMani linked an issue May 21, 2024 that may be closed by this pull request
@preetamrevankar preetamrevankar added this pull request to the merge queue May 22, 2024
Merged via the queue into main with commit 8fa2cd5 May 22, 2024
10 checks passed
@preetamrevankar preetamrevankar deleted the generate_access_code branch May 22, 2024 08:42
@ThisIsMani ThisIsMani restored the generate_access_code branch May 24, 2024 07:49
@ThisIsMani ThisIsMani deleted the generate_access_code branch May 24, 2024 07:51
@SanchithHegde SanchithHegde removed the S-waiting-on-review Status: This PR has been implemented and needs to be reviewed label May 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-users Area: Users C-feature Category: Feature request or enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

refactor: Separate out recovery codes from TOTP
5 participants