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

[CLOUDTRUST-2295] Bridge endpoint to self-ask for an email validation #183

Merged
merged 1 commit into from
Feb 25, 2020

Conversation

fperot74
Copy link
Contributor

[CLOUDTRUST-2296] Bridge endpoint to self-ask for a phone number validation
[CLOUDTRUST-2299] Send email when user updates his email/phone. Unvalidate his account validation

Refers to:

@coveralls
Copy link

coveralls commented Feb 19, 2020

Pull Request Test Coverage Report for Build 1874

  • 60 of 60 (100.0%) changed or added relevant lines in 5 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.07%) to 91.534%

Totals Coverage Status
Change from base Build 1873: 0.07%
Covered Lines: 4822
Relevant Lines: 5268

💛 - Coveralls

Gopkg.toml Outdated

[[constraint]]
name = "github.com/cloudtrust/keycloak-client"
branch = "dev"
branch = "jira-2296"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

To be updated before merging

@@ -15,6 +16,12 @@ import (
kc "github.com/cloudtrust/keycloak-client"
)

// Constants
const (
ActionVerifyEmail = "VERIFY_EMAIL"
Copy link
Contributor

Choose a reason for hiding this comment

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

why 2 diff styles?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes... one comes from Keycloak, the other one comes from 2019.kc-sms

@@ -27,6 +34,8 @@ type KeycloakAccountClient interface {
UpdateAccount(accessToken, realm string, user kc.UserRepresentation) error
GetAccount(accessToken, realm string) (kc.UserRepresentation, error)
DeleteAccount(accessToken, realm string) error
ExecuteActionsEmail(accessToken string, realmName string, actions []string) error
//ExecuteActionsEmail(accessToken string, actions []string, paramKV ...string) error
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe remove the second line

@@ -160,7 +160,22 @@ paths:
responses:
200:
description: Successful operation.

/account/send-verify-email:
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we could remove the "send" in URL to have /account/verify-email as it would sounds more RESTful

var additionalInfos = map[string]string{"actions": strings.Join(actions, ",")}
var additionalBytes, _ = json.Marshal(additionalInfos)
var additionalString = string(additionalBytes)
c.reportEvent(ctx, "ACTION_EMAIL", database.CtEventRealmName, currentRealm, database.CtEventUserID, userID, database.CtEventAdditionalInfo, additionalString)
Copy link
Contributor

Choose a reason for hiding this comment

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

ACTION_EMAIL must be sent only for VERIFY_EMAIL ? If yes, maybe we should have another specific event for verify phone number ?

@fratt-elca fratt-elca self-requested a review February 24, 2020 13:40
Copy link
Contributor

@fratt-elca fratt-elca left a comment

Choose a reason for hiding this comment

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

Retest OK!

@fperot74 fperot74 force-pushed the selfservice branch 2 times, most recently from 61fac01 to 0a67a1f Compare February 25, 2020 12:08
[CLOUDTRUST-2296] Bridge endpoint to self-ask for a phone number validation
[CLOUDTRUST-2299] Send email when user updates his email/phone. Unvalidate his account validation
@fperot74 fperot74 merged commit d974483 into cloudtrust:master Feb 25, 2020
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.

None yet

5 participants