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.

Sign up for GitHub

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

Jump to bottom

Feature/eyqb 319 no public access #182

Merged
merged 33 commits into from
Jun 14, 2024
Merged

Feature/eyqb 319 no public access #182

merged 33 commits into from
Jun 14, 2024

Conversation

RobertGHippo
Copy link
Collaborator

@RobertGHippo RobertGHippo commented Jun 11, 2024 •
edited
Loading

Description

All endpoints except Error and Health guarded behind configurable secrets. Can be turned off altogether through configuration, which will be done only in production once the service has achieved Authority to Operate.

There is an opportunity to extract this functionality into a reusable Nuget package if it is deemed more generally useful.

Ticket number (if applicable)

EYQB-319

How Has This Been Tested?

All runs as expected locally. Lots of new unit tests too. All end-to-end tests and accessibility tests pass also on the GitHub PR check action.

Screenshots

If user has not entered the secret, any page redirects them to a challenge page:
image

Once an acceptable key has been submitted on this page, the site works as if all the pages are public.

Checklist:

  • My code follows the standards used within this project
  • I have performed a self-review of my code
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests (Unit, E2E) that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

RobertGHippo and others added 30 commits June 5, 2024 12:25
@RobertGHippo
All controllers redirect to challenge url.
3848b74
@RobertGHippo
Simple cookie-based solution working with hard-coded secret.
0f28df9
@RobertGHippo
Postback not quite working yet
8e35a35
@RobertGHippo
Fix assertion
551496a
@RobertGHippo
Converted raw form to cshtml form helper.
b9e6621 
Only relative path addresses are considered "local" URLs.
@RobertGHippo
Allow multiple secret values.
c2cd1c2 
Secret values configurable.
@RobertGHippo
Only add the challenge filter if configured.
450fcd0 
New no-op "no challenge" filter if service access unchallenged.
@RobertGHippo
Test that only the correct controllers are unguarded.
236ce51 
Move Error endpoint to its own new unguarded controller.
@RobertGHippo
Filter passes through if public access allowed.
166cca1 
Doc comments (point to no-op filter for public access).
More tests.
@RobertGHippo
Don't cache the challenge response.
a349039
@RobertGHippo
Logger in the ErrorController is not used.
dfa8e17
@RobertGHippo
End-to-end tests now need an auth secret from env.
0f9b01a 
Will come from pipeline.
...or can come from command line on dev machines.
@RobertGHippo
Tf added gateway exclusion for auth-secret cookie.
e4a92c0 
Tf added variables for access challenge.
Tf configure service on deployment for access challenge.
Secret values will come from command line / pipeline.
@github-actions
terraform-docs: automated action
d9e220d
@RobertGHippo
First attempt at hooking up challenge to pipelines, e2e tests etc.
fbe970b
@RobertGHippo
Merge remote-tracking branch 'origin/feature/eyqb-319-no-public-acces…
b345f78 
…s' into feature/eyqb-319-no-public-access
@github-actions
terraform-docs: automated action
dc44afe
@RobertGHippo
Merge branch 'main' into feature/eyqb-319-no-public-access
eba3014
@RobertGHippo
Make the challenge box a little wider.
e598888
@RobertGHippo
Merge remote-tracking branch 'origin/feature/eyqb-319-no-public-acces…
ccab264 
…s' into feature/eyqb-319-no-public-access
@RobertGHippo
Investigate what happens if I pass secrets in.
65a54ed
@RobertGHippo
Secret parameter passing to run-app-for-testing too.
2735793
@RobertGHippo
Fix use of input
9cb3f95
@RobertGHippo
Another attempt at passing the secrets.
43867fa
@RobertGHippo
Screenshot zips are corrupt in v3: try v4 of the action.
9196432
@sam-c-dfe
Merge branch 'main' into feature/eyqb-319-no-public-access
0ebd91c
@RobertGHippo
Set a cookie on the Pa11y tests.
58cf097
@sam-c-dfe
Updated to add auth secret to pa11y config
6e599b7
@sam-c-dfe
Added js pa11y-ci implementation
add4048
@RobertGHippo
Deleted redundant config file.
35ae94e
@RobertGHippo RobertGHippo marked this pull request as ready for review June 13, 2024 13:10
@github-actions GitHub Actions
Copy link
Contributor

Code Coverage

Package Line Rate Branch Rate Health
Dfe.EarlyYearsQualification.Content 84% 91% ➖
Dfe.EarlyYearsQualification.Mock 91% 100% ✔
Dfe.EarlyYearsQualification.Web 96% 84% ✔
Summary 90% (1175 / 1307) 88% (139 / 158) ➖

Minimum allowed line rate is 75%

@RobertGHippo RobertGHippo merged commit 9367fb2 into main Jun 14, 2024
@RobertGHippo RobertGHippo deleted the feature/eyqb-319-no-public-access branch June 14, 2024 08:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sam-c-dfe sam-c-dfe sam-c-dfe approved these changes

@DanielClarkeEducation DanielClarkeEducation Awaiting requested review from DanielClarkeEducation

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@RobertGHippo @sam-c-dfe