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 direct push provider, fixes #534 #535

Merged

Conversation

lochiiconnectivity
Copy link
Contributor

@lochiiconnectivity lochiiconnectivity commented Mar 15, 2023

🔧 Changes

Add support for direct MFA push provider, fixes #534

📚 References

See #534

🔬 Testing

Use direct push provider

📝 Checklist

  • All new/changed/fixed functionality is covered by tests (or N/A)
  • I have added documentation for all new/changed functionality (or N/A)

@lochiiconnectivity lochiiconnectivity requested a review from a team as a code owner March 15, 2023 14:00
@sergiught
Copy link
Contributor

Hey @lochiiconnectivity 👋🏻

Thanks for raising this. At the moment the Auth0 Terraform Provider only supports managing Push Notification Providers of type "guardian" and "sns". The "direct" type is missing and support for this is blocked until it gets added within the Go SDK that this provider leverages.

I don't have an exact ETA of when we could add support for this within the Go SDK but contributions are more than welcomed there as well if you want to give it a try.

@sergiught sergiught closed this Mar 15, 2023
@lochiiconnectivity
Copy link
Contributor Author

Go SDK indicates that this is supported - https://github.com/auth0/go-auth0/blob/main/management/guardian.go#L339, this was added in auth0/go-auth0#136

@sergiught
Copy link
Contributor

Thanks for pointing that out @lochiiconnectivity, and apologies I should've been more precise. Although you can select the provider through the Go SDK using https://github.com/auth0/go-auth0/blob/main/management/guardian.go#L339, you can't configure the setting's for Push Notification that uses a Direct Provider because the endpoints are missing.

It's not enough to simply set the selected provider to "direct", in order to update these fields
image
We need support for the following endpoints:

@lochiiconnectivity
Copy link
Contributor Author

lochiiconnectivity commented Mar 15, 2023

See auth0/go-auth0#184 - if this moves forward, I will integrate support for the new SDK methods into this PR

@sergiught
Copy link
Contributor

That's awesome @lochiiconnectivity 🙇🏻 this is highly appreciated! I'll reopen this then and mark it as a draft instead.

@sergiught sergiught reopened this Mar 16, 2023
@sergiught sergiught marked this pull request as draft March 16, 2023 09:39
@lochiiconnectivity lochiiconnectivity force-pushed the issue#534/add-direct-push-provider branch from d68e807 to 2f7e607 Compare March 16, 2023 22:40
@lochiiconnectivity
Copy link
Contributor Author

Added content to enable the new functionality, but left placeholder references in go.mod and go.sum until the new SDK release, when that happens, will update with the final release version and hash

@sergiught sergiught force-pushed the issue#534/add-direct-push-provider branch from a8277ff to 570e1f6 Compare March 17, 2023 13:15
@sergiught
Copy link
Contributor

Thanks @lochiiconnectivity 👍🏻 , I rebased your PR and pointed go-auth0 to latest main so we're not blocked until we make the release to ensure this PR is in the right place.

@codecov-commenter
Copy link

Codecov Report

Patch coverage: 77.06% and project coverage change: -0.08 ⚠️

Comparison is base (73204bf) 84.26% compared to head (570e1f6) 84.18%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #535      +/-   ##
==========================================
- Coverage   84.26%   84.18%   -0.08%     
==========================================
  Files          78       78              
  Lines       10186    10293     +107     
==========================================
+ Hits         8583     8665      +82     
- Misses       1295     1317      +22     
- Partials      308      311       +3     
Impacted Files Coverage Δ
internal/auth0/guardian/expand.go 69.17% <38.88%> (-4.49%) ⬇️
internal/auth0/guardian/flatten.go 75.13% <84.21%> (+1.06%) ⬆️
internal/auth0/guardian/resource.go 94.40% <100.00%> (+0.67%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@sergiught sergiught force-pushed the issue#534/add-direct-push-provider branch from 570e1f6 to cce063c Compare April 19, 2023 17:38
@sergiught sergiught marked this pull request as ready for review April 19, 2023 17:41
Copy link
Contributor

@sergiught sergiught left a comment

Choose a reason for hiding this comment

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

Awesome work @lochiiconnectivity 💯 , we cut a release on the Auth0 Go SDK and I rebased this PR along with a few very minor tweaks. Thanks a lot for the contribution!

@sergiught sergiught requested a review from willvedd April 19, 2023 17:42
@sergiught sergiught merged commit f08e9d8 into auth0:main Apr 21, 2023
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.

MFA Push CustomApp should be type 'direct' and not 'guardian'
3 participants