Skip to content

Create the auth "vertical" #714

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

Merged
merged 3 commits into from
Aug 23, 2023
Merged

Create the auth "vertical" #714

merged 3 commits into from
Aug 23, 2023

Conversation

agateau-gg
Copy link
Collaborator

Intro

This PR introduces an auth "vertical". It's not a real vertical but it provides the same support as other ggshield.verticals packages to the ggshield.cmd packages.

What has been done

I initially considered moving part of cmd.login and cmd.logout there but it would require more refactoring to make the code less tied to the CLI parsing and output. It is not worth the move without these refactoring IMHO and I do not want to spend time on it for now.

Instead I just moved OAuthClient and its related code there, and moved an helper date formatting function to core.textutils.

While I was at it, I simplified said helper date and made it output a more correctly formatted string.

Ref

Part of #521.

@codecov-commenter
Copy link

codecov-commenter commented Aug 22, 2023

Codecov Report

Merging #714 (2be5838) into main (efac953) will increase coverage by 0.00%.
Report is 2 commits behind head on main.
The diff coverage is 100.00%.

❗ 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     #714   +/-   ##
=======================================
  Coverage   92.20%   92.21%           
=======================================
  Files         135      136    +1     
  Lines        5839     5841    +2     
=======================================
+ Hits         5384     5386    +2     
  Misses        455      455           
Flag Coverage Δ
unittests 92.21% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
ggshield/cmd/auth/login.py 96.38% <100.00%> (ø)
ggshield/core/text_utils.py 100.00% <100.00%> (ø)
ggshield/verticals/auth/__init__.py 100.00% <100.00%> (ø)
ggshield/verticals/auth/oauth.py 86.17% <100.00%> (ø)

Copy link
Contributor

@fbochu fbochu left a comment

Choose a reason for hiding this comment

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

LGTM

@agateau-gg agateau-gg merged commit efc8a3f into main Aug 23, 2023
@agateau-gg agateau-gg deleted the agateau/reorg-auth branch August 23, 2023 15:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants