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(Auth): Add TOTP Support (#2537) #2568

Merged
merged 22 commits into from
Aug 29, 2023
Merged

feat(Auth): Add TOTP Support (#2537) #2568

merged 22 commits into from
Aug 29, 2023

Conversation

gpanshu
Copy link
Contributor

@gpanshu gpanshu commented Aug 22, 2023

  • PR title and description conform to Pull Request guidelines.

Issue #, if available:

Description of changes:

How did you test these changes?
(Please add a line here how the changes were tested)

Documentation update required?

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@gpanshu gpanshu requested a review from a team as a code owner August 22, 2023 17:33
@gpanshu gpanshu self-assigned this Aug 22, 2023
@gpanshu gpanshu enabled auto-merge (squash) August 23, 2023 17:08
@gpanshu gpanshu disabled auto-merge August 23, 2023 17:09
@codecov-commenter
Copy link

codecov-commenter commented Aug 23, 2023

Codecov Report

Merging #2568 (77f3793) into main (aac074a) will decrease coverage by 0.27%.
The diff coverage is 30.86%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@            Coverage Diff             @@
##             main    #2568      +/-   ##
==========================================
- Coverage   41.50%   41.23%   -0.27%     
==========================================
  Files         874      885      +11     
  Lines       27643    28240     +597     
  Branches     3883     3993     +110     
==========================================
+ Hits        11473    11645     +172     
- Misses      14944    15312     +368     
- Partials     1226     1283      +57     

@gpanshu gpanshu enabled auto-merge (squash) August 24, 2023 15:31
ankpshah
ankpshah previously approved these changes Aug 26, 2023
Copy link
Member

@tylerjroach tylerjroach left a comment

Choose a reason for hiding this comment

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

Feature tests are missing

@@ -681,6 +683,53 @@ class AWSCognitoAuthPluginTest {
verify(timeout = CHANNEL_TIMEOUT) { realPlugin.clearFederationToIdentityPool(any(), any()) }
}

@Test
fun setUpTOTP() {
Copy link
Member

Choose a reason for hiding this comment

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

For this test and the 3 tests below it, any() shouldn't be used on the last blocks. Its should be asserting that the correct listeners, etc are passed. In this case it should check expectedOnSuccess and expectedOnError.

tylerjroach
tylerjroach previously approved these changes Aug 29, 2023
@gpanshu gpanshu merged commit 19cc6e4 into main Aug 29, 2023
3 checks passed
@gpanshu gpanshu deleted the feature/totp branch August 29, 2023 21:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants