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

Fix 70: Role Urls in Tokens should be kept unchanged upon creating / parsing tokens. #71

Merged
merged 12 commits into from
Nov 29, 2019

Conversation

shimono
Copy link
Member

@shimono shimono commented Nov 19, 2019

Role Url in tokens should be as follows:

Token Role Info currently Shoud fix
TransCellAccessToken Role Class URL Role Instance URL Yes
ResidentAccessToken - - No
ResidentRefreshToken - - No
GrantCode - - No
VisitorAccessToken Role Instance URL Role Instance URL No
VisitorRefreshToken Role Class URL Role Instance URL Yes

This fix is necessary to fix the following problem of personium-core
personium/personium-core#516

@shimono shimono requested a review from tochi-y November 19, 2019 18:08
@shimono shimono self-assigned this Nov 19, 2019
@shimono shimono added this to In progress in Current Work via automation Nov 19, 2019
Co-Authored-By: Tochiori Yausufmi <32325140+tochi-y@users.noreply.github.com>
@shimono shimono moved this from In progress to Review in progress in Current Work Nov 27, 2019
@shimono shimono changed the title Feature 70 Feature 70: Role Url in TransCellAccessToken should be Role class URL Nov 27, 2019
@shimono shimono changed the title Feature 70: Role Url in TransCellAccessToken should be Role class URL Feature 70: Role Url in TranceCellAccessToken/VisitorRefreshToken should be Role Class URL. Nov 27, 2019
@tochi-y
Copy link
Member

tochi-y commented Nov 29, 2019

@shimono this PR is still working? If it's working, add WIP to the title.

@shimono shimono changed the title Feature 70: Role Url in TranceCellAccessToken/VisitorRefreshToken should be Role Class URL. Fix 70: Role Urls in TranceCellAccessToken should be kept unchanged upon creating / parsing tokens. Nov 29, 2019
@shimono shimono changed the title Fix 70: Role Urls in TranceCellAccessToken should be kept unchanged upon creating / parsing tokens. WIP: Fix 70: Role Urls in Tokens should be kept unchanged upon creating / parsing tokens. Nov 29, 2019
…ocking tests to minimize the use of mocking.

Probably, the target method itself (refreshAccessToken) is useless. It can easily be substituted with AccessToken Constructor.
@shimono shimono changed the title WIP: Fix 70: Role Urls in Tokens should be kept unchanged upon creating / parsing tokens. Fix 70: Role Urls in Tokens should be kept unchanged upon creating / parsing tokens. Nov 29, 2019
@shimono
Copy link
Member Author

shimono commented Nov 29, 2019

Let me merge this PR to dev branch for the following reasons.

  • The change made here is related to the other PR. (PR for Fixing #73 #74)
  • Tests in the other PR is useful to test the change made in this PR.
  • We need to use the updated lib-common with both 2 PR to fix the bug in core.
  • We also need to release engine using new lib-common (1.5.3)

@shimono shimono merged commit 8d13c5f into personium:develop Nov 29, 2019
Current Work automation moved this from Review in progress to Done Nov 29, 2019
@shimono shimono mentioned this pull request Nov 29, 2019
@tochi-y tochi-y added this to the 1.5.3 milestone Dec 9, 2019
@tochi-y tochi-y added the bug label Dec 9, 2019
@@ -159,7 +147,8 @@ public static VisitorRefreshToken parse(final String token, final String issuer)
String[] extra = ret.populate(token.substring(PREFIX_TC_REFRESH.length()), issuer, 3);
ret.id = extra[IDX_ID];
ret.originalIssuer = extra[IDX_ORIG_ISSUER];
ret.roleList = AbstractOAuth2Token.parseRolesString(extra[IDX_ORIG_ROLE_LIST]);
// Role class url list is there in Visitor Refresh Tokens
ret.roleList = AbstractOAuth2Token.parseSpaceSeparatedRoleClassUrlString(extra[IDX_ORIG_ROLE_LIST]);
return ret;
} catch (MalformedURLException e) {
throw AbstractOAuth2Token.PARSE_EXCEPTION;
Copy link
Member

Choose a reason for hiding this comment

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

Better to include MalformedURLException to TokenParseException.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Current Work
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants