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

cleanup and merge all Verifier variations #314

Closed
4 tasks done
muhlemmer opened this issue Mar 8, 2023 · 3 comments · Fixed by #336
Closed
4 tasks done

cleanup and merge all Verifier variations #314

muhlemmer opened this issue Mar 8, 2023 · 3 comments · Fixed by #336
Assignees
Milestone

Comments

@muhlemmer
Copy link
Collaborator

muhlemmer commented Mar 8, 2023

As a maintainer I want to increase test coverage, however I'm lazy and don't want to do this for redundant code.

Situation of Verifier types

Currently we have a base interface oidc.Verifier:

type Verifier interface {
Issuer() string
MaxAgeIAT() time.Duration
Offset() time.Duration
}

Which is further extended in specific interface type:

Then, each of the above variations has a private struct type and a constructor function. (For the sake of simplicity I will not link those here.)

Proposal

The Verifiers are mere data carriers with reference parameters we use to check against. The actual verification is done in a related VerifyXXXToken function which passes relevant parameters to CheckXXX functions in oidc. Therefore I would make sense to merge all of the above types in a single struct type on oidc and delete all the getter methods.

The constructor function for specific tokens will be kept to guide users into setting the correct fields.

Acceptance criteria

  • oidc.Verifier will be a struct type
  • The fields currently in all the op and rp types are merged into oidc.Verifier
  • The oidc.Verifier fields will be exported
  • The op and rp types will be removed
@muhlemmer muhlemmer added enhancement New feature or request backend labels Mar 8, 2023
@muhlemmer
Copy link
Collaborator Author

Also related to zitadel/zitadel#5232

@muhlemmer muhlemmer self-assigned this Mar 16, 2023
@muhlemmer muhlemmer linked a pull request Mar 21, 2023 that will close this issue
3 tasks
@github-actions
Copy link

🎉 This issue has been resolved in version 3.0.0-next.2 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@muhlemmer muhlemmer added this to the v3 milestone Apr 22, 2023
@github-actions
Copy link

🎉 This issue has been resolved in version 3.0.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant