Skip to content

RDM-4369: Support reserved characters in userId email address #420

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

Merged
merged 15 commits into from
Apr 15, 2019
Merged

Conversation

Daniel-Lam
Copy link
Contributor

JIRA link (if applicable)

https://tools.hmcts.net/jira/browse/RDM-4369.

Change description

Ensure GET request to /user-profile/users endpoint uses a fully encoded URL, to allow reserved characters in the uid query parameter.

Does this PR introduce a breaking change? (check one with "x")

[x] Yes
[ ] No

Note: PR hmcts/ccd-user-profile-api#173 must be merged to master before this change; User Profile API needs to be able to decode the encoded email address passed in the uid query parameter, as a result of this PR.

Ensure `GET` request to `/user-profile/users` endpoint uses a fully encoded URL, to allow reserved characters in the `uid` query parameter.
@Daniel-Lam Daniel-Lam added the checkpoint Checkpoint Review with QA label Mar 18, 2019
@Daniel-Lam Daniel-Lam requested a review from tzarsmango March 18, 2019 21:06
Switch to using `DefaultUriBuilderFactory` in Spring Boot 2 to encode the request URL, to fix a problem with "double encoding" of query parameters (see spring-projects/spring-framework#20750 (comment)).
@codecov
Copy link

codecov bot commented Mar 19, 2019

Codecov Report

Merging #420 into master will increase coverage by 0.07%.
The diff coverage is 80%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #420      +/-   ##
==========================================
+ Coverage   95.28%   95.35%   +0.07%     
==========================================
  Files         295      295              
  Lines        6676     6672       -4     
  Branches      389      387       -2     
==========================================
+ Hits         6361     6362       +1     
+ Misses        315      310       -5
Impacted Files Coverage Δ
.../main/java/uk/gov/hmcts/ccd/ApplicationParams.java 77.55% <ø> (ø) ⬆️
...gov/hmcts/ccd/data/user/DefaultUserRepository.java 96.42% <80%> (+10.15%) ⬆️
...v/hmcts/ccd/domain/model/definition/FieldType.java 97.05% <0%> (-0.51%) ⬇️
.../domain/model/aggregated/CaseViewFieldBuilder.java 100% <0%> (ø) ⬆️
...v/hmcts/ccd/domain/model/definition/CaseField.java 100% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d2b4c1d...227438a. Read the comment docs.

@jenkins-reform-hmcts2 jenkins-reform-hmcts2 temporarily deployed to preview March 19, 2019 15:13 Inactive
@jenkins-reform-hmcts2 jenkins-reform-hmcts2 requested a deployment to preview March 19, 2019 15:17 Abandoned
@jenkins-reform-hmcts2 jenkins-reform-hmcts2 temporarily deployed to preview March 19, 2019 15:38 Inactive
@jenkins-reform-hmcts2 jenkins-reform-hmcts2 temporarily deployed to preview March 19, 2019 16:14 Inactive
@Daniel-Lam Daniel-Lam requested a review from gbenadikar March 19, 2019 16:31
@jenkins-reform-hmcts2 jenkins-reform-hmcts2 temporarily deployed to preview March 19, 2019 16:35 Inactive
Copy link
Contributor

@tzarsmango tzarsmango left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@gbenadikar gbenadikar left a comment

Choose a reason for hiding this comment

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

A comment

@jenkins-reform-hmcts2 jenkins-reform-hmcts2 temporarily deployed to preview March 19, 2019 18:00 Inactive
@jenkins-reform-hmcts2 jenkins-reform-hmcts2 temporarily deployed to preview March 19, 2019 22:07 Inactive
…tring parameter

Avoid setting the encoding mode on `restTemplate` because this is an autowired singleton bean, shared across the application. A future ticket will address encoding query string parameters for *all* requests, using `restTemplate`.
Copy link
Contributor

@gbenadikar gbenadikar left a comment

Choose a reason for hiding this comment

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

👍

@jenkins-reform-hmcts2 jenkins-reform-hmcts2 temporarily deployed to preview March 21, 2019 11:15 Inactive
@jenkins-reform-hmcts2 jenkins-reform-hmcts2 temporarily deployed to preview March 25, 2019 16:27 Inactive
@jenkins-reform-hmcts2 jenkins-reform-hmcts2 temporarily deployed to preview March 25, 2019 20:40 Inactive
Copy link
Contributor

@mario-paniccia mario-paniccia left a comment

Choose a reason for hiding this comment

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

👍

@jenkins-reform-hmcts2 jenkins-reform-hmcts2 temporarily deployed to preview March 29, 2019 15:39 Inactive
@jenkins-reform-hmcts2 jenkins-reform-hmcts2 temporarily deployed to preview April 1, 2019 13:24 Inactive
@jenkins-reform-hmcts2 jenkins-reform-hmcts2 temporarily deployed to preview April 9, 2019 11:19 Inactive
@jenkins-reform-hmcts2 jenkins-reform-hmcts2 temporarily deployed to preview April 10, 2019 10:16 Inactive
@jenkins-reform-hmcts2 jenkins-reform-hmcts2 temporarily deployed to preview April 11, 2019 12:49 Inactive
@codecov-io
Copy link

codecov-io commented Apr 11, 2019

Codecov Report

Merging #420 into master will increase coverage by 0.07%.
The diff coverage is 80%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #420      +/-   ##
==========================================
+ Coverage   95.28%   95.35%   +0.07%     
==========================================
  Files         295      295              
  Lines        6676     6681       +5     
  Branches      389      389              
==========================================
+ Hits         6361     6371      +10     
+ Misses        315      310       -5
Impacted Files Coverage Δ
.../main/java/uk/gov/hmcts/ccd/ApplicationParams.java 77.55% <ø> (ø) ⬆️
...gov/hmcts/ccd/data/user/DefaultUserRepository.java 96.42% <80%> (+10.15%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0eb92fe...fd4d7f5. Read the comment docs.

@jenkins-reform-hmcts2 jenkins-reform-hmcts2 temporarily deployed to preview April 15, 2019 16:24 Inactive
@hemantt hemantt merged commit bcd4a51 into master Apr 15, 2019
@delete-merged-branch delete-merged-branch bot deleted the RDM-4369 branch April 15, 2019 16:32
mohammedanas added a commit that referenced this pull request Apr 30, 2019
* master:
  RDM-4542: Use lowercase email for User Profile retrieval (#452)
  FR-901
  Revert "RDM-3325: White space validation rules applied on text fields (#284)" (#459)
  RDM-4610: CVE-2019-0232 fix (#458)
  increased default callback timeout from 15sec to 60 sec
  Rdm 3897- Retrieve all the case data (#376)
  RDM-3325: White space validation rules applied on text fields (#284)
  RDM-4369: Support reserved characters in userId email address (#420)
  Add externalId index for CMC (#418)
  Fix OWASP issue (#456)
  SIDAM Switch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
checkpoint Checkpoint Review with QA
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants