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

Add OpenID Connect integration #9

Merged
merged 30 commits into from
Sep 23, 2024
Merged

Add OpenID Connect integration #9

merged 30 commits into from
Sep 23, 2024

Conversation

weiiwang01
Copy link
Collaborator

Add OpenID Connect integration to the Penpot charm using the oauth charm library.

Since SMTP is required when OpenID is enabled in Penpot, SMTP integration is now a requirement when enabling OAuth.

Integration tests for oauth have been added using oauth_tools from the identity-bundle. However, as there are some issues currently with the oauth_tools, please withhold merging this pull request until the problems have been fixed upstream.

@weiiwang01 weiiwang01 requested a review from a team as a code owner August 16, 2024 06:50
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

license-eye has checked 46 files.

Valid Invalid Ignored Fixed
12 1 33 0
Click to see the invalid file list
  • tests/integration/prepare.sh
Use this command to fix any missing license headers
```bash

docker run -it --rm -v $(pwd):/github/workspace apache/skywalking-eyes header fix

</details>

tests/integration/prepare.sh Show resolved Hide resolved
weiiwang01 and others added 4 commits August 16, 2024 07:35
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Thanhphan1147
Thanhphan1147 previously approved these changes Aug 19, 2024
Copy link

@Thanhphan1147 Thanhphan1147 left a comment

Choose a reason for hiding this comment

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

LGTM, I only have a few minor comments.

src/charm.py Outdated Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
yanksyoon
yanksyoon previously approved these changes Aug 26, 2024
Copy link

@yanksyoon yanksyoon left a comment

Choose a reason for hiding this comment

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

LGTM! Few questions, thank you!

penpot_rock/rockcraft.yaml Show resolved Hide resolved
src/charm.py Show resolved Hide resolved
penpot_rock/rockcraft.yaml Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
yanksyoon
yanksyoon previously approved these changes Sep 9, 2024
Copy link

@yanksyoon yanksyoon left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you!

src/charm.py Show resolved Hide resolved
Copy link

@javierdelapuente javierdelapuente left a comment

Choose a reason for hiding this comment

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

LGTM

src/charm.py Show resolved Hide resolved
src/charm.py Show resolved Hide resolved
tests/integration/test_charm.py Outdated Show resolved Hide resolved
tests/integration/test_charm.py Show resolved Hide resolved
Copy link

Test coverage for 7eb2df9

Name           Stmts   Miss Branch BrPart  Cover   Missing
----------------------------------------------------------
src/charm.py     238     29     76     17    85%   85-86, 98-100, 114-115, 124-126, 136-138, 147, 154-155, 254, 273, 302, 334, 350, 356, 358, 372, 400, 419, 433, 470, 515
----------------------------------------------------------
TOTAL            238     29     76     17    85%

Static code analysis report

Run started:2024-09-23 06:28:13.602031

Test results:
  No issues identified.

Code scanned:
  Total lines of code: 1205
  Total lines skipped (#nosec): 1
  Total potential issues skipped due to specifically being disabled (e.g., #nosec BXXX): 0

Run metrics:
  Total issues (by severity):
  	Undefined: 0
  	Low: 0
  	Medium: 0
  	High: 0
  Total issues (by confidence):
  	Undefined: 0
  	Low: 0
  	Medium: 0
  	High: 0
Files skipped (0):

@weiiwang01 weiiwang01 merged commit 76d9c96 into main Sep 23, 2024
17 checks passed
@weiiwang01 weiiwang01 deleted the add-oauth branch September 23, 2024 10:16
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.

5 participants