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

Consolidate login.gov url #218

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

jmdembe
Copy link

@jmdembe jmdembe commented Aug 9, 2023

Changes proposed in this pull request:

Current state: login.gov and www.login.gov are two valid and separate URLs that look like they are the same website. This PR will work to consolidate the URLs--a user will hit login.gov no matter what URL that they use.
C

Security considerations

None

@jmdembe jmdembe force-pushed the login-redirect-revisit branch from eebaf9e to cde64e0 Compare August 9, 2023 15:13
pages.yml Outdated
Comment on lines 52 to 53
- from: www.login.gov
to: login.gov
Copy link
Contributor

@aduth aduth Aug 10, 2023

Choose a reason for hiding this comment

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

We want the reverse, to redirect from login.gov to www.login.gov

Or at least we've been using www version as canonical in the site itself and in links generating on the IdP.

@@ -496,6 +496,14 @@ server {
return 301 https://$target_domain;
}

# www.login.gov to login.gov
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# www.login.gov to login.gov
# login.gov to www.login.gov

@@ -30,6 +30,7 @@ const expectedRedirects = [
{ from: 'usability.gov', to: 'www.usability.gov', redirectCode: 301 },
{ from: 'emerging.digital.gov', to: 'digital.gov/topics/emerging-tech', redirectCode: 301, noPath: true },
{ from: 'components.designsystem.digital.gov', to: 'designsystem.digital.gov/components', redirectCode: 301, noPath: true },
{ from: 'www.login.gov', to: 'login.gov', redirectCode: 301}
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like a syntax error with missing comma

Suggested change
{ from: 'www.login.gov', to: 'login.gov', redirectCode: 301}
{ from: 'www.login.gov', to: 'login.gov', redirectCode: 301 },

Comment on lines +52 to +54
- from: login.gov
to: www.login.gov
toDomain: login.gov
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this needed? Stands out to me that toDomain is the legacy domain. Should it be the new domain?

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