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

wrap admin and preview in pan-domain-authentication 🐼 #27012

Merged
merged 3 commits into from
May 2, 2024

Conversation

twrichards
Copy link
Contributor

@twrichards twrichards commented Apr 3, 2024

Prerequisites

What is the value of this and can you measure success?

Prior to this PR the admin and preview apps within frontend were wrapped in plain Google auth (since they require a Guardian login to use) - this leaves us at the mercy of Google outages (as occurred recently on 5th March for example). This PR replaces Google auth with our own pan-domain-authentication which wraps Google auth with a handy extra feature to make us resilient to such outages.

This also tees us up nicely for adding permissions to admin and preview in #27078

What does this change?

Overall this is NET reduction in code 🎉 and a reasonable amount of the diff is splitting/moving things around. A summary of the changes is as follows...

  • add nginx/nginx-mapping.yml for use with dev-nginx so that admin and preview can run on frontend.local.dev-gutools.co.uk and preview.local.dev-gutools.co.uk respectively - so that the pan-domain-authentication actually works (won't work when running purely on localhost)
  • renames GoogleAuthFilters in common, to the more fitting GuardianAuthWithExemptions
  • replaces the com.gu.play-googleauth lib with pan-domain-auth-play_3-0 (and removes the now unnecessary com.gu.play-secret-rotation and com.gu.play-secret-rotation)
  • initialises instances of GuardianAuthWithExemptions in admin and preview's AppLoaders
  • within GuardianAuthWithExemptions
  • removes AdminAuthAction replaces uses of it with regular Action since the filter should be enforcing auth more broadly
  • strips out login buttons and endpoints from admin and preview since panda signs in / redirects automatically

Screenshots

N/A

Checklist

project/Dependencies.scala Outdated Show resolved Hide resolved
@twrichards twrichards marked this pull request as ready for review May 1, 2024 09:22
@twrichards twrichards requested a review from a team as a code owner May 1, 2024 09:22
twrichards added a commit to guardian/editorial-viewer that referenced this pull request May 1, 2024
Copy link
Contributor

@mxdvl mxdvl left a comment

Choose a reason for hiding this comment

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

Looks good 🐼

Some questions and comments for my own education, but nothing blocking I could spot

)(wsClient, executionContext)
def healthCheck(): Action[AnyContent] =
Action {
Ok("OK")
Copy link
Contributor

Choose a reason for hiding this comment

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

0K

Comment on lines +59 to +61
.map(new URI(_))
.map(_.getPath)
.map(DigestUtils.md5Hex)
Copy link
Contributor

Choose a reason for hiding this comment

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

unroll please 🧵 👏

admin/app/views/admin_main.scala.html Show resolved Hide resolved
@@ -1,6 +1,15 @@
import app.FrontendComponents
import play.api.ApplicationLoader.Context
import play.api.BuiltInComponentsFromContext
import play.api.mvc.EssentialFilter
import org.scalatestplus.mockito.MockitoSugar
Copy link
Contributor

Choose a reason for hiding this comment

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

Mockito Sugar, yum 😋

dev-nginx restart-nginx

echo -e "💯 Done! You can now run frontend locally on https://${DOMAIN}"
echo -e "You can also (separately) run admin and preview locally on https://frontend.local.dev-gutools.co.uk and https://preview.local.dev-gutools.co.uk respectively."
Copy link
Contributor

Choose a reason for hiding this comment

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

🔐

nginx/frontend.conf Show resolved Hide resolved
preview/app/http/PreviewNoCacheFilter.scala Show resolved Hide resolved
@twrichards twrichards merged commit 3873b15 into main May 2, 2024
4 checks passed
@twrichards twrichards deleted the pan-domain-authentication branch May 2, 2024 09:27
@prout-bot
Copy link
Collaborator

Seen on FRONTS-PROD, ADMIN-PROD (merged by @twrichards 14 minutes and 10 seconds ago)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants