AuthPermissions.AspNetCore icon indicating copy to clipboard operation
AuthPermissions.AspNetCore copied to clipboard

Published 20 hours ago • verified publisher icon JonPSmith

Added an option for expiration to user invitation

Open idan-h opened this issue 2 years ago • 3 comments

Hey, added an option for expiration to InviteNewUserService: IInviteNewUserService
Tests confirmed

idan-h avatar Jul 13 '22 18:07 idan-h

Also, I noticed there is no check if the roles are valid for the user? From what I have seen, a user can invite another user with app admin role? If so, this needs to be addressed.. would happy to hear your thoughts

idan-h avatar Jul 13 '22 19:07 idan-h

Hi @idan-h,

I noticed there is no check if the roles are valid for the user

That isn't correct. This section in the InviteNewUserService checks that invites to a tenant user have their roles checked.

JonPSmith avatar Jul 14 '22 08:07 JonPSmith

Hi @idan-h,

I noticed there is no check if the roles are valid for the user

That isn't correct. This section in the InviteNewUserService checks that invites to a tenant user have their roles checked.

yea, I see it now.. Idk how I missed that. Removed the faulty commits.

idan-h avatar Jul 17 '22 10:07 idan-h

Hi @idan-h,

I looked at your PR and it didn't link to the AddNewUserDto and didn't include any code to set the expiration. I therefore in version 3.4.0 I added the code to make it more part of the invite a user feature. I appreciated your suggestion and I thing the invite user feature is better for your input.

JonPSmith avatar Sep 15 '22 12:09 JonPSmith

Hey, what do you mean by "didn't include any code to set the expiration"? I Added an option to add expiration time to the method CreateInviteUserToJoinAsync by adding a new dto AddNewUserWithExpirationDto and added a check when adding the user.

You can see the test I wrote to understand better

idan-h avatar Sep 15 '22 12:09 idan-h

If you want to change the current dto instead of what I did it is fine by me, but I think that the expiration option is a must in this case (atleast for me).. I wouldn't want a user joining with a link I sent after monthes

idan-h avatar Sep 15 '22 13:09 idan-h

Hey, what do you mean by "didn't include any code to set the expiration"?

Sorry, you didn't create a frontend way to set the expiration time. It wasn't that hard to do, but I always want the examples use all the features so that developers how it works. See example in Example3 and Example5.

I appreciate you help and suggestions but it is my decision whether I take a PR. Your idea was really good and the ReleaseNotes contains a (inspired by @idan-h) note against the expiration addition. The idea was yours and I just wrote the code in a way that was more part of the invite user code.

JonPSmith avatar Sep 15 '22 13:09 JonPSmith

Hey, what do you mean by "didn't include any code to set the expiration"?

Sorry, you didn't create a frontend way to set the expiration time. It wasn't that hard to do, but I always want the examples use all the features so that developers how it works. See example in Example3 and Example5.

I appreciate you help and suggestions but it is my decision whether I take a PR. Your idea was really good and the ReleaseNotes contains a (inspired by @idan-h) note against the expiration addition. The idea was yours and I just wrote the code in a way that was more part of the invite user code.

Oh, I do not care much about the credit and it wasn't my intention. Sorry if it came that way.

I thought you didn't recognize the need for this option so you didn't include it. My bad.

Anyway, I meant for the expiration to be set by the back-side only (as it is a security measure). But sure, some examples won't hurt.

idan-h avatar Sep 15 '22 14:09 idan-h