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

issue-1589 #28

Merged
merged 3 commits into from
Apr 7, 2022
Merged

issue-1589 #28

merged 3 commits into from
Apr 7, 2022

Conversation

robfusion
Copy link
Contributor

@robfusion robfusion commented Apr 4, 2022

FusionAuth/fusionauth-issues#1589

Add link and unlink events
Comment on lines 53 to 56
UserIdentityProviderLink("user.idp.link"),

UserIdentityProviderUnlink("user.idp.unlink"),

Copy link
Member

Choose a reason for hiding this comment

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

  1. Please alphabetize these
  2. I wonder if we should spell out idp - other events such as user.two-factor.method.add use the long form and mostly matches the Enum name. Would it be too long to do user.identity-provider.link and user.identity-provider.unlink?

@@ -0,0 +1,53 @@
/*
* Copyright (c) 2019-2022, FusionAuth, All Rights Reserved
Copy link
Member

Choose a reason for hiding this comment

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

This is a new class, should be 2022.

*
* @author Rob Davis
*/
public class UserIdentityProviderLinkEvent extends BaseEvent implements Buildable<UserIdentityProviderLinkEvent>, NonTransactionalEvent {
Copy link
Member

Choose a reason for hiding this comment

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

Missing equals and hashCode.

public UserIdentityProviderLinkEvent() {
}

public UserIdentityProviderLinkEvent(EventInfo info, String identityProviderName, User user) {
Copy link
Member

Choose a reason for hiding this comment

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

It is fine if we add the name, but the event needs to for sure contain the IdP Id. It seems that we should also include details about the link itself. Should we be adding the entire link object? Or at least the IdP user unique Id, IdP user display name if we have it, the FusionAuth IdP link Id, etc. We need to assume someone wants to consume this event and then use it to create FKs, or call back to FusionAuth and be able to retrieve specific information.

@@ -0,0 +1,53 @@
/*
* Copyright (c) 2019-2022, FusionAuth, All Rights Reserved
Copy link
Member

Choose a reason for hiding this comment

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

2022

@@ -1,5 +1,5 @@
/*
* Copyright (c) 2018-2019, FusionAuth, All Rights Reserved
* Copyright (c) 2018-2022, FusionAuth, All Rights Reserved
Copy link
Member

Choose a reason for hiding this comment

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

Looks like we have the wrong copyright here, can you update this to the Apache license.

@@ -1,5 +1,5 @@
/*
* Copyright (c) 2018-2020, FusionAuth, All Rights Reserved
* Copyright (c) 2018-2022, FusionAuth, All Rights Reserved
Copy link
Member

Choose a reason for hiding this comment

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

Looks like we have the wrong copyright here, can you update this to the Apache license.

*
* @author Rob Davis
*/
public class UserIdentityProviderUnlinkEvent extends BaseEvent implements Buildable<UserIdentityProviderUnlinkEvent>, NonTransactionalEvent {
Copy link
Member

Choose a reason for hiding this comment

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

Equals, hashcode.

public UserIdentityProviderUnlinkEvent() {
}

public UserIdentityProviderUnlinkEvent(EventInfo info, String identityProviderName, User user) {
Copy link
Member

Choose a reason for hiding this comment

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

Same comment form the link event, we should add more details to this event.


@Override
public EventType getType() {
return EventType.UserIdentityProviderLink;
Copy link
Member

Choose a reason for hiding this comment

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

This is the wrong type.

robfusion and others added 2 commits April 6, 2022 11:32
FusionAuth/fusionauth-issues#1589

New Event Types for Identity Provider Link and Unlink
…oken for some tests. Fix method naming in LastLoginMapper. Add more tests.

Add a test for end-to-end workflows for Device Grant and IdP linking with the new Link Events.
@robfusion robfusion merged commit 650923c into master Apr 7, 2022
@johnjeffers johnjeffers deleted the rob/issue-1589 branch May 30, 2024 04:10
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.

2 participants