-
Notifications
You must be signed in to change notification settings - Fork 7
refactor and add m2m token support for user endpoints #11
Conversation
| if(expirySeconds!=null) | ||
| jwt.setExpirySeconds(expirySeconds); | ||
| List<String> roles = new ArrayList<String>(); | ||
| List<String> roles = new ArrayList<>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reason for taking out this type-checking?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mtwomey just clean up, Java is language feature means this part can be omitted, since we have List<String> https://docs.oracle.com/javase/tutorial/java/generics/genTypeInference.html
mtwomey
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Kindly review comments
| * Represents the update scopes for machine token validation. | ||
| */ | ||
| public static final String[] UpdateScopes = {"update:user_profiles", "all:user_profiles"}; | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a place we could put these so they're not hard coded?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mtwomey not sure I understand this, you want to make these configurable like using environment files?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@skyhit I'm not a fan of magic strings either. We're going to accept the PR but can you follow up with another PR to pull all the magic strings out to the config and inject via env vars with defaults?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cwdcwd ok, I will make my suggestion.
| // to handle fake social account, adding below check before verifiying from auth0 | ||
| if(profile.getContext()==null) { | ||
| profile.setContext(new HashMap<String, String>()); | ||
| profile.setContext(new HashMap<>()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed type-checking?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mtwomey just clean up, Java 8's new language feature means this part can be omitted. https://docs.oracle.com/javase/tutorial/java/generics/genTypeInference.html
src/main/java/com/appirio/tech/core/service/identity/resource/UserResource.java
Show resolved
Hide resolved
| if(expirySeconds!=null) | ||
| jwt.setExpirySeconds(expirySeconds); | ||
| List<String> roles = new ArrayList<String>(); | ||
| List<String> roles = new ArrayList<>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.
No description provided.