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

Possible to trigger TOTP activation and login using TOTP #259

Merged
merged 28 commits into from
May 13, 2022

Conversation

alneberg
Copy link
Contributor

@alneberg alneberg commented Feb 24, 2022

Before submitting a pr:

  • Tests passing
  • Black formatting
  • Rebase/merge the dev branch
  • Note in the CHANGELOG

@i-oden i-oden marked this pull request as draft March 4, 2022 08:40
@aanil aanil marked this pull request as ready for review April 20, 2022 14:29
CHANGELOG.md Outdated Show resolved Hide resolved
dds_cli/__main__.py Outdated Show resolved Hide resolved
dds_cli/__main__.py Outdated Show resolved Hide resolved
dds_cli/__main__.py Outdated Show resolved Hide resolved
dds_cli/user.py Outdated Show resolved Hide resolved
dds_cli/user.py Outdated Show resolved Hide resolved
dds_cli/__main__.py Show resolved Hide resolved
Copy link
Member

@i-oden i-oden left a comment

Choose a reason for hiding this comment

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

Seems to be a bug somewhere. The one-time code is never accepted.

root@a119c8420857:/code# dds auth twofactor
     ︵ 
 ︵ (  )   ︵ 
(  ) ) (  (  )   SciLifeLab Data Delivery System 
 ︶  (  ) ) (    http://dds_backend:5000/ 
      ︶ (  )    Version 1.0.3 
          ︶
INFO     Starting configuration of one-time authentication code method.                                         
? Which method would you like to use? Authenticator App
INFO     No saved token found, or token has expired, proceeding with authentication                             
DDS username: unituser_1
DDS password: 
INFO     Please enter the one-time authentication code sent to your email address (leave empty to exit):        
Authentication one-time code: 14367889
Authentication one-time code: 14367889
Authentication one-time code: 

@i-oden
Copy link
Member

i-oden commented May 5, 2022

@alneberg @aanil Also, all requests now go through the perform_request function in the utils.py module

@alneberg
Copy link
Contributor Author

alneberg commented May 5, 2022

Yes, the bug seems to be due to an indentation misstake in the merge with dev. Should be easy to fix.

@codecov
Copy link

codecov bot commented May 11, 2022

Codecov Report

Merging #259 (e9b465b) into dev (52f331b) will decrease coverage by 0.01%.
The diff coverage is 2.98%.

@@           Coverage Diff            @@
##             dev    #259      +/-   ##
========================================
- Coverage   7.17%   7.15%   -0.02%     
========================================
  Files         29      29              
  Lines       2440    2487      +47     
========================================
+ Hits         175     178       +3     
- Misses      2265    2309      +44     
Impacted Files Coverage Δ
dds_cli/__main__.py 0.00% <0.00%> (ø)
dds_cli/auth.py 0.00% <0.00%> (ø)
dds_cli/base.py 0.00% <ø> (ø)
dds_cli/user.py 0.00% <0.00%> (ø)
dds_cli/__init__.py 91.66% <100.00%> (+0.23%) ⬆️
dds_cli/utils.py 44.82% <0.00%> (+0.57%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 52f331b...e9b465b. Read the comment docs.

@i-oden
Copy link
Member

i-oden commented May 12, 2022

@alneberg @aanil You can add the following to a codecov.yml file in the repo root, this is the same as we have in the dds_web

coverage:
  status:
    project:
      default:
        threshold: 5%
    patch: off

@i-oden
Copy link
Member

i-oden commented May 12, 2022

@zishanmirza commented the following in the Slack dds-development channel:

I noticed the cli doesn't have tests for TOTP. Would be great to add that to the PR.

Copy link
Member

@i-oden i-oden left a comment

Choose a reason for hiding this comment

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

Works well! If you could add a couple of tests though, that would be great! You don't have to add heaps, but look at the existing tests regarding perform_request and a couple new ones for this.

Also: Changelog

CHANGELOG.md Outdated Show resolved Hide resolved
Copy link
Member

@i-oden i-oden left a comment

Choose a reason for hiding this comment

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

Looks great here too!

@i-oden i-oden merged commit 0b45a8a into ScilifelabDataCentre:dev May 13, 2022
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.

3 participants