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

fix: custom stylesheet and js are not loaded after OIDC authentication #544

Conversation

FabienArcellier
Copy link
Collaborator

@FabienArcellier FabienArcellier commented Aug 28, 2024

fix #535

The problem was not fully solved, and worse, has broke the OIDC authentication mechanism. The access to /static url is blocked after user authentication. I fail to diagnose this issue because I was loading those assets through browser cache in local during my test.

image

The problem is visible on dev review environment. It is fixed on this PR review environment.


  • load writer framework assets
  • load user assets into statics
  • load user extensions

@FabienArcellier FabienArcellier self-assigned this Aug 28, 2024
@FabienArcellier FabienArcellier added the bug Something isn't working label Aug 28, 2024
@FabienArcellier FabienArcellier force-pushed the 535-custom-stylesheet-and-js-are-not-loaded-after-oidc-authentication-2 branch from aaef770 to e77d369 Compare August 28, 2024 12:39
@FabienArcellier FabienArcellier marked this pull request as ready for review August 28, 2024 12:48
@FabienArcellier FabienArcellier marked this pull request as draft August 28, 2024 19:50
@@ -191,14 +191,15 @@ def register(self,
redirect_url = urljoin(self.host_url, self.callback_authorize)
host_url_path = urlpath(self.host_url)
callback_authorize_path = urljoin(host_url_path, self.callback_authorize)
static_assets_path = urljoin(host_url_path, "static")

auth_ignored_prefix_paths = [urljoin(host_url_path, "static"), urljoin(host_url_path, "assets")]
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I will use the same approach than the code used in server to scan the static asset into the repo

@ramedina86
Copy link
Collaborator

  • Can't we make auth work for all paths instead of ignoring paths? There's nothing that says static/ is public, and exposing assets/ doesn't seem necessary either.
  • Have you tested custom components (/extensions)?

@FabienArcellier
Copy link
Collaborator Author

FabienArcellier commented Aug 30, 2024

I already investigate protecting static assets with WF's stateless architecture without finding a good solution. In the end, it is a common technique not to protect them, particularly to allow their management by a CDN.

The management of browser session cookies, which is supposed to be able to allow this, has been hijacked by browsers. They are persisted today. If the user refreshes the page, opens a new tab, closes and reopens the browser, the session cookie is retained.

Session cookies — cookies without a Max-Age or Expires attribute – are deleted when the current session ends. The browser defines when the "current session" ends, and some browsers use session restoring when restarting. This can cause session cookies to last indefinitely.

Pasted image 20240830083835

Strangely, the mechanism works better for basic auth because the browser forgets the credentials when they are closed. It's also a header propagate to all the request (as cookie).

Here is the two topics where I have been stuck. Do you see a way to go further without dropping the stateless current architecture ?

inject custom header for every query to the server. There remains a major difficulty if we give javascript the right to access the session identifier. How to make the browser add a header to all requests, including static asset loading through an img tag. I'm stuck, it's supposed to be the role of cookies to do that. I don't know of a way to make a cookie used by a single tab. We return to the initial problem.

I look to the contextual cookie technique. It does not make it possible to manage this type of case. It is used by Google for multi-account. It only works for APIs, not for static asset loading.

@FabienArcellier
Copy link
Collaborator Author

FabienArcellier commented Aug 30, 2024

To fix the current issue, I'm hesitating between 2 strategies. Either add a blacklist in the authentication configuration, or add a whitelist in the authentication. The safest is the whitelist. It's easy to document but hard to diagnose when problem happens.

writer.auth.Oidc(
	protect_static_assets=[
		'assets' 
		'dataset/private_asset.csv'
	]
)
writer.auth.Oidc(
	public_static_assets=[
		'/assets' # expose the files inside the static/assets folder
		'/dataset/asset.csv', # expose the files inside the static/dataset/asset.csv
	]
)

@FabienArcellier
Copy link
Collaborator Author

FabienArcellier commented Aug 30, 2024

/extensions has the same problem.

@FabienArcellier FabienArcellier force-pushed the 535-custom-stylesheet-and-js-are-not-loaded-after-oidc-authentication-2 branch 2 times, most recently from 4d26dcf to 2d23b3c Compare August 30, 2024 07:49
* fix: allow access to assets and static endpoint
* docs: improve details about authentication
@FabienArcellier FabienArcellier force-pushed the 535-custom-stylesheet-and-js-are-not-loaded-after-oidc-authentication-2 branch from 2d23b3c to a6af9f7 Compare September 1, 2024 09:26
@FabienArcellier FabienArcellier marked this pull request as ready for review September 1, 2024 13:52
@ramedina86 ramedina86 merged commit 0471fdb into writer:dev Sep 3, 2024
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

custom stylesheet and js are not loaded after OIDC authentication
2 participants