Skip to content

feat(saml): implementation of saml for ZITADEL v2#3618

Merged
fforootd merged 76 commits intomainfrom
v2-saml
Sep 12, 2022
Merged

feat(saml): implementation of saml for ZITADEL v2#3618
fforootd merged 76 commits intomainfrom
v2-saml

Conversation

@stebenz
Copy link
Copy Markdown
Contributor

@stebenz stebenz commented May 12, 2022 •
edited by peintnermax
Loading

Bildschirmfoto 2022-06-16 um 08 44 53

@stebenz stebenz added enhancement New feature or request lang: go Pull requests that update Go code backend labels May 12, 2022
@stebenz stebenz added this to the SAML 2.0 IDP Support milestone May 12, 2022
@stebenz stebenz requested a review from livio-a May 12, 2022 09:06
@stebenz stebenz self-assigned this May 12, 2022
@fforootd
Copy link
Copy Markdown
Member

fforootd commented May 12, 2022

It has some conflicts can you have a look on those?

@stebenz
Copy link
Copy Markdown
Contributor Author

stebenz commented May 12, 2022 •
edited
Loading

It has some conflicts can you have a look on those?

Yeah I just now merged back the v2 branch to clear up this conflicts.
There were some conflicts with the proto files where I had to manually resolve each element as the formatting in the branches was different so it took some time.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 12, 2022 •
edited
Loading

Codecov Report

Merging #3618 (8646d3d) into main (8ab85af) will decrease coverage by 0.23%.
The diff coverage is 33.05%.

@@            Coverage Diff             @@
##             main    #3618      +/-   ##
==========================================
- Coverage   47.11%   46.88%   -0.24%     
==========================================
  Files         622      628       +6     
  Lines       55204    56133     +929     
==========================================
+ Hits        26012    26319     +307     
- Misses      27894    28490     +596     
- Partials     1298     1324      +26
Impacted Files Coverage Δ
...nternal/api/grpc/management/project_application.go 0.00% <0.00%> (ø)
...i/grpc/management/project_application_converter.go 0.00% <0.00%> (ø)
internal/command/command.go 5.66% <0.00%> (-0.47%) ⬇️
internal/command/key_pair.go 0.00% <0.00%> (ø)
internal/command/key_pair_model.go 0.00% <0.00%> (ø)
internal/command/unique_constraints_model.go 0.00% <0.00%> (ø)
internal/crypto/rsa.go 0.00% <0.00%> (ø)
internal/domain/application_saml.go 0.00% <0.00%> (ø)
internal/domain/auth_request.go 0.00% <0.00%> (ø)
internal/domain/key_pair.go 0.00% <0.00%> (ø)
... and 19 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@fforootd fforootd changed the title feat(saml): impementation of saml for ZITADEL v2 feat(saml): implementation of saml for ZITADEL v2 May 21, 2022
@netlify
Copy link
Copy Markdown

netlify Bot commented Jun 13, 2022 •
edited
Loading

✅ Deploy Preview for docs-zitadel-com canceled.

Name Link
🔨 Latest commit 8646d3d
🔍 Latest deploy log https://app.netlify.com/sites/docs-zitadel-com/deploys/631f4f6adf34f100088aaecc

livio-a
livio-a requested changes Jul 20, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@livio-a livio-a left a comment

Choose a reason for hiding this comment

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

i did a first review... before i check everything can you please go through the comments i've made
i've also teste locally and was not able to create an app using the metadata url
@peintnermax maybe you have to check this, because it did work when using the api directly
but the url is never returned @stebenz

Comment thread cmd/defaults.yaml Outdated
Comment thread proto/zitadel/app.proto Outdated
Comment thread proto/zitadel/management.proto Outdated
Comment thread proto/zitadel/management.proto Outdated
Comment thread proto/zitadel/app.proto Outdated
Comment thread docs/docs/guides/saml/auth0.md Outdated
Comment thread docs/docs/guides/saml/auth0.md Outdated
Comment thread internal/crypto/rsa.go
Comment thread internal/domain/application_saml.go
Comment thread internal/domain/application_saml.go Outdated
eliobischof
eliobischof previously requested changes Sep 8, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@eliobischof eliobischof left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Please add an overview API card to the docs in /docs/apis/introduction

image

mffap
mffap requested changes Sep 8, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@mffap mffap left a comment

Choose a reason for hiding this comment

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

Critical imo would be the urls in the guides. Rest are more QoL improvements.

Comment thread docs/docs/apis/saml/endpoints.md Outdated
Comment thread docs/docs/apis/saml/endpoints.md Outdated
Comment thread docs/docs/apis/saml/endpoints.md Outdated
Comment thread docs/docs/apis/saml/endpoints.md Outdated
Comment thread docs/docs/apis/saml/endpoints.md Outdated
Comment thread docs/docs/guides/integrate/auth0-saml.md Outdated
Comment thread docs/docs/guides/integrate/aws-saml.md Outdated
Comment thread docs/docs/guides/integrate/aws-saml.md Outdated
Comment thread docs/docs/guides/integrate/pingidentity-saml.md Outdated
Comment thread docs/sidebars.js Outdated
mffap
mffap requested changes Sep 8, 2022
View reviewed changes
Comment thread docs/docs/guides/integrate/pingidentity-saml.md Outdated
@mffap
Copy link
Copy Markdown
Member

mffap commented Sep 9, 2022

@stebenz please update the readme: feature section. SAML 2.0 should ideally point to the docs

mffap
mffap reviewed Sep 9, 2022
View reviewed changes
Comment thread README.md Outdated
stebenz and others added 3 commits September 9, 2022 14:06
livio-a
livio-a previously requested changes Sep 9, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@livio-a livio-a left a comment

Choose a reason for hiding this comment

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

i know there are already some i18n keys missing, but could you please add the both newly added to all 5 language files:
EventTypes.project.application.config.saml.added and EventTypes.project.application.config.saml.changed

Comment thread .releaserc.js Outdated
branches: [
{name: 'main'},
{name: '1.87.x', range: '1.87.x', channel: '1.87.x'},
{name: 'v2-saml', prerelease: 'beta'},
Copy link
Copy Markdown
Member

@livio-a livio-a Sep 9, 2022

Choose a reason for hiding this comment

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

just a reminder to remove this line as soon as we don't need a new prerelease anymore

@stebenz
Copy link
Copy Markdown
Contributor Author

stebenz commented Sep 12, 2022

Please add an overview API card to the docs in /docs/apis/introduction

image

As there is also no OIDC part, is it really necessary? Endpoints are described, otherwise, we should include OIDC here as well.

@eliobischof
Copy link
Copy Markdown
Contributor

eliobischof commented Sep 12, 2022

Please add an overview API card to the docs in /docs/apis/introduction

image

As there is also no OIDC part, is it really necessary? Endpoints are described, otherwise, we should include OIDC here as well.

Didn't think so far 😄 fine by me

mffap
mffap previously approved these changes Sep 12, 2022
View reviewed changes
@fforootd fforootd dismissed stale reviews from livio-a and eliobischof September 12, 2022 15:19

Because he is in vacation!

fforootd
fforootd reviewed Sep 12, 2022
View reviewed changes
Comment thread .releaserc.js Outdated
@fforootd fforootd merged commit 7a5f7f8 into main Sep 12, 2022
@fforootd fforootd deleted the v2-saml branch September 12, 2022 16:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fforootd fforootd fforootd approved these changes

@mffap mffap mffap left review comments

@livio-a livio-a livio-a left review comments

+1 more reviewer

@eliobischof eliobischof eliobischof left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@stebenz stebenz

Labels

backend enhancement New feature or request lang: go Pull requests that update Go code

Projects

None yet

Milestone

SAML 2.0 IDP Support

6 participants

@stebenz @fforootd @mffap @eliobischof @livio-a @peintnermax