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

Replace basic auth with a custom authentication process #1182

Merged
merged 1 commit into from
Apr 5, 2022

Conversation

joelanman
Copy link
Contributor

@joelanman joelanman commented Nov 25, 2021

Browsers no longer always support basic auth so this PR replaces it with:

  • middleware to check req.session.authentication is the current encrypted password
  • if not, redirect to a new prototype-admin/password page
  • a route to check the password POSTed from the page matches the PASSWORD set in the environment
  • if so, set req.session.authentication to the current encrypted password
  • redirect back to the page initially requested

to test this branch, run:

NODE_ENV=production USE_HTTPS=false PASSWORD=test npm start

Screenshot:
This is a prototype used for research. It is not a real service. You should only continue if you have been invited to test this prototype. Password text input, Continue button

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-prototype-kit-pr-1182 November 25, 2021 12:13 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-prototype-kit-pr-1182 November 25, 2021 12:41 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-prototype-kit-pr-1182 November 25, 2021 16:17 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-prototype-kit-pr-1182 November 25, 2021 18:12 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-prototype-kit-pr-1182 November 26, 2021 14:04 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-prototype-kit-pr-1182 January 26, 2022 16:58 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-prototype-kit-pr-1182 January 26, 2022 18:21 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-prototype-kit-pr-1182 February 9, 2022 10:53 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-prototype-kit-pr-1182 February 9, 2022 12:03 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-prototype-kit-pr-1182 February 9, 2022 14:58 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-prototype-kit-pr-1182 February 9, 2022 17:55 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-prototype-kit-pr-1182 February 11, 2022 10:30 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-prototype-kit-pr-1182 February 11, 2022 15:27 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-prototype-kit-pr-1182 February 11, 2022 15:41 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-prototype-kit-pr-1182 February 11, 2022 17:32 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-prototype-kit-pr-1182 February 16, 2022 10:41 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-prototype-kit-pr-1182 February 16, 2022 11:05 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-prototype-kit-pr-1182 February 16, 2022 11:22 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-prototype-kit-pr-1182 February 16, 2022 12:12 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-prototype-kit-pr-1182 February 16, 2022 12:29 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-prototype-kit-pr-1182 February 16, 2022 12:31 Inactive
@joelanman joelanman marked this pull request as ready for review February 17, 2022 12:05
@trang-erskine trang-erskine linked an issue Feb 22, 2022 that may be closed by this pull request
@joelanman
Copy link
Contributor Author

joelanman commented Feb 23, 2022

@ma3574

Hi, is there any further info on "Browsers no longer always support basic auth" as the link in the first comment just links back to this pr? Thanks :)

sorry don't know what happened there, fixed now:
#1019

@joelanman
Copy link
Contributor Author

discussed with Laurence - we could render the password page without redirecting, using res.render. This would mean the password page does not disrupt the browser history. Something to discuss with @natcarey

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-prototype-kit-pr-1182 March 1, 2022 17:21 Inactive
@joelanman
Copy link
Contributor Author

joelanman commented Mar 1, 2022

updated with:

  • underscore in input name (_password) so the password is not auto-stored
  • 10 character password input width (in practice its more like 20 characters and theres no actual limit to the number of character)
  • load Frontend JS and initialise it, to focus the Error Summary
  • use govuk-grid-column-two-thirds-from-desktop for better use of screen width on tablets

@lfdebrux
Copy link
Member

lfdebrux commented Mar 2, 2022

One other thought, we could salt the password. Currently two sites with the same password will have the same cookie value, not a massive issue but is slightly bad practice? Also, if we have a salt, that gives a way to revoke access without changing passwords, which may or may not be useful. Any thoughts on this @natcarey?

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-prototype-kit-pr-1182 March 2, 2022 15:10 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-prototype-kit-pr-1182 March 10, 2022 15:43 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-prototype-kit-pr-1182 March 11, 2022 11:53 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-prototype-kit-pr-1182 March 23, 2022 11:15 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-prototype-kit-pr-1182 March 23, 2022 11:52 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-prototype-kit-pr-1182 March 31, 2022 14:22 Inactive
@nataliecarey
Copy link
Contributor

@lfdebrux I've just been scanning back through and seen the salt question. I think I brushed it aside before because there's no data store containing the password but actually we could include a salt either:

  • A uuid in a config file which is generated and then committed
  • Something derived from the service name

Would you see value in either of those?

@lfdebrux
Copy link
Member

@natcarey the first thing was more what I was thinking, because it allows changing easily if necessary. If we don't think it is necessary though that is fine.

@joelanman
Copy link
Contributor Author

joelanman commented Mar 31, 2022

I don't have a strong opinion on salt, @natcarey should lead on it. I think you'd need access to someone's cookies to know that both encrypted passwords are the same, so it doesn't feel like a likely scenario. Committing it to git would make it public in many cases no?

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-prototype-kit-pr-1182 April 5, 2022 14:23 Inactive
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.

Basic auth does not always work
6 participants