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

UML-3121 make redirect uri dynamic #2390

Merged
merged 13 commits into from
Nov 3, 2023
Merged

Conversation

MishNajam
Copy link
Contributor

@MishNajam MishNajam commented Oct 30, 2023

Purpose

Make redirect_uri use the correct domain depending on environments

Fixes UML-3121

Approach

Uses the Mezzio serverUrlHelper and urlHelper classes in service-front and passes this down to service-api.

Learning

The $redirectUrl being passed from the frontend to the api needs to match the redirect_url set in the docker dependencies file.
Selecting Welsh modifies the URL by adding '/cy/'. We have removed this from the redirectUrl to make this match.
The ui_locale value is stored in the AuthSession to allow returning from one login to a welsh page if selected.

Checklist

  • I have performed a self-review of my own code
  • I have added relevant logging with appropriate levels to my code
  • New event_codes have been documented on the wiki page
  • I have updated documentation (Confluence/GitHub wiki/tech debt doc) where relevant
  • I have added tests to prove my work
  • I have added welsh translation tags and updated translation files
  • I have run an accessibility tool on any pages I have made changes to and fixed any issues found
  • I have notified the Interaction Designer of any content changes so that appropriate screenshots/flow diagram changes can be made
  • The product team have tested these changes

@MishNajam MishNajam requested a review from a team as a code owner October 30, 2023 14:54
@codecov
Copy link

codecov bot commented Oct 30, 2023

Codecov Report

Merging #2390 (4e050db) into main (e140372) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##               main    #2390   +/-   ##
=========================================
  Coverage     92.03%   92.03%           
  Complexity     1463     1463           
=========================================
  Files           279      279           
  Lines          6655     6656    +1     
=========================================
+ Hits           6125     6126    +1     
  Misses          513      513           
  Partials         17       17           
Flag Coverage Δ
use-an-lpa-admin 88.81% <ø> (ø)
use-an-lpa-api 96.84% <100.00%> (+0.06%) ⬆️
use-an-lpa-front 89.22% <100.00%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
...ntication/OneLoginAuthenticationRequestService.php 100.00% <100.00%> (ø)
...rc/Common/src/Service/OneLogin/OneLoginService.php 100.00% <100.00%> (ø)

@MishNajam MishNajam force-pushed the UML-3121-dynamic-redirect-uri branch from 9aa1b12 to f88dd04 Compare October 31, 2023 15:07
Scenario: I initiate authentication via one login in Welsh
Given I am on the temporary one login page
And I request to view the content in welsh
When I click the one login button in welsh
Copy link
Contributor

Choose a reason for hiding this comment

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

This should ideally be When I click the login button. The assertion on the language would then be in the Then.

The usefulness of the context files is that a step can create state that a later step can access. In this case you'd want to set the locale in the context when you request to view the content in welsh.

@MishNajam MishNajam merged commit 46c77d1 into main Nov 3, 2023
32 checks passed
@MishNajam MishNajam deleted the UML-3121-dynamic-redirect-uri branch November 3, 2023 12:27
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