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

feat: set basic jwt authentication allowlist #1596

Merged
merged 28 commits into from
Oct 7, 2024
Merged

Conversation

abvthecity
Copy link
Contributor

@abvthecity abvthecity commented Oct 3, 2024

Pruning the navigation tree

The pages that do not require authentication are allowed to be cached, and thus they do get served via SSG. In this case, the navigation tree needs to be pruned to show ONLY the pages that unauthenticated users are allowed to see.

This required adding the following:

  1. a BFS traverser where:
  2. if the visited node should be deleted, it will be removed from its parent, and
  3. if the node cannot be deleted from the parent, then it will be removed as well, and
  4. if the parent node no longer contains any children (or content), also delete it (recursively)
  5. finally, update all the pointsTo redirects after pruning the tree, so that redirects only point to public pages.

In other words: pruning applies to all leaf nodes (visitable in the browser). But, if any container (i.e. section, api reference, changelog, etc) is found to NOT contain children, it too will be deleted.

Special handling for section overview pages:

If a section contains an overview page, but it is not included in the allowlist, then only the overview page itself will get deleted. But since section nodes are not leaf nodes and may contain children, we avoid deleting it unless it has no children left.

Known edge case:

if there's a stream/nonstream endpoint node (aka. EndpointPairNode), if only one of the two nodes are marked to be pruned, then both nodes will be pruned

Applying auth on routes selectively

Adds the ability to set an allowlist and a denylist on pages and api routes using edge config, when the authentication scheme is set to basic_token_verification.

To be conservative, the denylist is prioritized over the allowlist, and anything that isn't captured in the allowlist will automatically be denied as well.

The allowlist/denylist leverages path-to-regexp@6 which allows wildcard and regex matching.

Some API routes must be explicitly allowed. for example:

  • /sitemap.xml
  • /api/fern-docs/sitemap.xml (must be set on both the origin and rewrite)
  • /path/to/changelog.rss
  • /api/fern-docs/changelog (must be set on both the origin and rewrite)
  • /api/fern-docs/api-definition/[api]/endpoint/[endpoint] (danger if certain endpoints are enabled, the api routes also need to be explicitly enabled or else api playground will stop working).
  • /api/fern-docs/search (danger. this will enable search. if enabled, all records will be searchable from algolia publicly.)

UPDATE (10/7): 8222166 undos the strict auth checks applied to all of the api endpoints except for /api/fern-docs/search. Two reason:

  1. sitemap and changelog can be pruned out of the box
  2. api definitions require knowledge of the ApiDefinitionId and EndpointId, and FDR doesn't gate these requests upstream. We can update this to be more clever at gating specific endpoints in the future by utilizing the pruner; but we'd need to cache the navigation nodes otherwise this endpoint will become extraordinarily slow.

What's next

  • there should be a feature flag that, when set, instead of outright deleting all the authenticated pages, it should mark them as "locked" but still visible in the frontend.
  • enable audience filters using the pruning mechanism
  • enable launchdarkly feature flags using the pruning mechanism
  • move the custom auth config (allowlist) into docs.yml
  • segment algolia search records between authenticated/unauthenticated, and other audiences.

Tests

"allowlist": [
  "/",
  "/my-section/public-page",
  "/api-reference/imdb/get-movie"
]
Screenshot 2024-10-04 at 5 09 45 PM

Copy link

github-actions bot commented Oct 3, 2024

Playwright test results

passed  1 passed

Details

stats  1 test across 1 suite
duration  9 seconds
commit  5ab1320

@abvthecity abvthecity had a problem deploying to Preview - app.buildwithfern.com October 3, 2024 22:25 — with GitHub Actions Failure
@abvthecity abvthecity had a problem deploying to Preview - app-dev.buildwithfern.com October 3, 2024 22:25 — with GitHub Actions Failure
@abvthecity abvthecity marked this pull request as ready for review October 4, 2024 01:10
@abvthecity abvthecity requested a review from dsinghvi as a code owner October 4, 2024 01:10
@abvthecity abvthecity had a problem deploying to Preview - app.buildwithfern.com October 4, 2024 01:11 — with GitHub Actions Failure
@abvthecity abvthecity had a problem deploying to Preview - app-dev.buildwithfern.com October 4, 2024 01:11 — with GitHub Actions Failure
@abvthecity abvthecity had a problem deploying to Preview - app-dev.buildwithfern.com October 4, 2024 01:23 — with GitHub Actions Failure
@abvthecity abvthecity had a problem deploying to Preview - app.buildwithfern.com October 4, 2024 01:23 — with GitHub Actions Failure
Copy link
Member

@dsinghvi dsinghvi left a comment

Choose a reason for hiding this comment

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

I think that we will need to productize this pretty fast (i.e. support audiences in docs.yml, multiple search indexes with the upcoming docs deployments)

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.

None yet

2 participants