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

Site tweaks to improve strict CSP hosting #2492

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

Site tweaks to improve strict CSP hosting #2492

wants to merge 7 commits into from

Conversation

rajsite
Copy link
Member

@rajsite rajsite commented Dec 10, 2024

Pull Request

🤨 Rationale

The reason for this PR was trying to evaluate Nimble-based apps hosting in Web App hosting under our strict CSP policy. This was done by exercising the nimble site build which covers the following: Angular, Blazor, Vite, Storybook.

In doing so identified some changes that are either useful or minimally invasive as workarounds for WebApp hosting issues or improving the experience of Nimble site if hosted in Web App hosting.

👩‍💻 Implementation

Vite:

  • A minimal change to provide a relative base configuration for vite was needed. This causes vite to change from paths relative to root (/script.js) to relative paths in the current directory (./script.js). Reason is WebApps are not hosted in root but from a subdirectory path.

Angular:

  • A minimal change to disable inlineCritical styles is needed so that stylesheets included in angular.json load correctly (which seems to be something we figured out and inherited in all apps but doesn't seem to be documented anywhere 🤷 it's like a herd immunity inherited in copy-paste generations 💉). Reason is described in blog / we don't allow unsafe-inline.

Blazor:

  • During investigation found that Blazor does not like to serve from index.html urls and relies on path urls. You can actually see it on the current published site if you use a blazor index.html style url instead of a directory url. The page fails to load and has many console errors as Blazor does poor URL parsing / manipulation to load JS resources. It's probably worth creating an issue but I did not yet.
  • I thought of a workaround for nimble site by specifying a base url manually of <base href="./" />. This is not compatible with our strict CSP setting and is ignored due to base-uri: 'none';'' (the OWASP strict policy example) causing the errors to continue. We potentially could switch our CSP to base-uri: 'self' but it's not clear to me what the security implications are and I'm not recommending it yet (see following).
  • I also found that Blazor seems generally against the idea of serving off index.html vs the directory path as index.html resolution is not supported out of the box in the Router either.
  • I thought of a Router workaround for index.html resolution by serving the same component from both the path url and the index.html url. It seems to work but I'm not aware of any other concerns relying on that workaround.
  • Even with the above a Blazor 8 app using Nimble will still not run as some Blazor 8 template binding features require using eval and are only addressed in Blazor 9.
  • Based on the above, while I'll propose the changes for Nimble's Blazor site page to support index.html based urls working, I'm not going to propose changes to Web App hosting strict CSP base-uri configuration to support the workaround described above. Instead, I think we should say Nimble Blazor WebApp hosting is contingent on Blazor 9 support in Nimble and fixing path serving in Web App hosting. I don't think we should recommend the workarounds I figured out above to Blazor WebApp devs as from the linked issues Blazor does not seem interested in supporting that pattern.

Storybook:

  • Depends on unsafe-inline (which we do not allow) and they seem resistant to the change. Need to reply on the issue with a convincing discussion, not sure if MDN and OWASP recommendations are sufficient for them. Did not provide a comment / create an issue.

All:

  • Updated links to point to index.html paths as it's minimally invasive (makes the URL bar uglier) but works around AzDo 2941644. This could be reverted in Nimble once the WebApp directory hosting issue is address.
  • Updated each page to have a link to the parent page to make them easier to navigate between when hosted in the WebApp hosting iframe.

🧪 Testing

Manual and via built storybook.
Also example hosted on dev:

✅ Checklist

  • I have updated the project documentation to reflect my changes or determined no changes are needed.

Blazor partially addressed by still need .net 9 changes for full nimble support: dotnet/aspnetcore#58322 (comment)
@rajsite rajsite changed the title Landing and angular tweaks Site tweaks to improve strict CSP hosting Dec 10, 2024
@rajsite rajsite marked this pull request as ready for review December 11, 2024 04:32
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.

1 participant