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

website/integrations: Pocketbase #12906

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

Conversation

nicedevil007
Copy link
Contributor

Heya @4d62,

here is my try on getting the "regular" PocketBase Documentation done.
This was suggested as better usecase for authentik vs. PocketBase in general than just creating the same documenation for all Pocketbase applications over and over again.

With this guide we can reference all those applications to this one here :)

This was done prior to this guide where the idea came up: #12905


Checklist

  • Local tests pass (ak test authentik/)
  • The code has been formatted (make lint-fix)

If an API change has been made

  • The API schema has been updated (make gen-build)

If changes to the frontend have been made

  • [] The code has been formatted (make web)

If applicable

  • The documentation has been updated
  • The documentation has been formatted (make website)

@nicedevil007 nicedevil007 requested a review from a team as a code owner January 31, 2025 21:37
Copy link

netlify bot commented Jan 31, 2025

Deploy Preview for authentik-docs ready!

Name Link
🔨 Latest commit 458720b
🔍 Latest deploy log https://app.netlify.com/sites/authentik-docs/deploys/679f355a309e4c00086ce34e
😎 Deploy Preview https://deploy-preview-12906--authentik-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Jan 31, 2025

Deploy Preview for authentik-storybook canceled.

Name Link
🔨 Latest commit 458720b
🔍 Latest deploy log https://app.netlify.com/sites/authentik-storybook/deploys/679f355afbfa230007995e9a

Copy link

codecov bot commented Jan 31, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.67%. Comparing base (7c944b9) to head (458720b).
Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #12906      +/-   ##
==========================================
- Coverage   92.75%   92.67%   -0.09%     
==========================================
  Files         769      770       +1     
  Lines       38929    38945      +16     
==========================================
- Hits        36109    36092      -17     
- Misses       2820     2853      +33     
Flag Coverage Δ
e2e 48.46% <ø> (-0.11%) ⬇️
integration 24.59% <ø> (-0.02%) ⬇️
unit 90.37% <ø> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@dominic-r dominic-r left a comment

Choose a reason for hiding this comment

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

Hi @nicedevil007, thanks for your contibution. It is greatly appreciated. Here are a few things I noticed. Please do let me know if you have any questions.

This documentation lists only the settings that you need to change from their default values. Be aware that any changes other than those explicitly mentioned in this guide could cause issues accessing your application.
:::

## authentik configuration
Copy link
Contributor

Choose a reason for hiding this comment

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

Note to tana: This should be left as-is for now and I'll update format /button names later on.


Take note of the Client ID and Client Secret, you'll need to give them to PocketBase later.

- Redirect URIs/Origins (RegEx): `https://_pocketbase.company_/api/oauth2-redirect`
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see anything in this redirect URI that would warrant it to be set to "regex" instead of "strict". It's best to set it to "strict" unless absolutely needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi and thank you for your Review. Maybe it my wrong understanding of the Wiki about contribution but I used all "placeholder variables" in italic Font. I switched between different Write options now through my last PRs and Never know what is wanted and what Not.

Can someone clear this up?

  • bold stuff for things on the SP side that can be found on the gui f.e.
  • itatlic for placeholders

That is what I understood so far. Now it seems to be yet another new formatation here. I get more and more confused.

Copy link
Contributor

Choose a reason for hiding this comment

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

The Fonts & Fonts styling section of our style guide gives an overview of what's expected. What do you mean by "new formatation here"?

Italic is to be used for variables, yes. Bold for stuff found in the GUI, yes. URLs should be in a codeblock and variables in them should be italicized and the easiest way to do that is the suggestion I gave.

Copy link
Contributor Author

@nicedevil007 nicedevil007 Feb 1, 2025

Choose a reason for hiding this comment

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

I mean this Part of the Guide

Use italic font for variables or placeholders to make it clear they need to be replaced. Choose placeholder names that highlight their purpose, ensuring users understand what to update.


Thank you for making this clear now :) So I can Go oder my last PRs to Match this Styling :) Will take a Bit of time, but I Like have it "the same" on everything

Copy link
Contributor

@dominic-r dominic-r Feb 1, 2025

Choose a reason for hiding this comment

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

Could you also address the original review comment? Thanks

I don't see anything in this redirect URI that would warrant it to be set to "regex" instead of "strict". It's best to set it to "strict" unless absolutely needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe I copy Pasted wrong line and it was Late at Night. Will get to it tomorrow. But I guess you are right :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So I always wrote it like the menu entry is in authentik:
image

"Redirect URIs/Origins (RegEx)" is the setting we have to set. But it doesn't mean that we should choose RegEx on the dropdown, so how should I do it now?

website/integrations/services/pocketbase/index.mdx Outdated Show resolved Hide resolved
website/integrations/services/pocketbase/index.mdx Outdated Show resolved Hide resolved
website/integrations/services/pocketbase/index.mdx Outdated Show resolved Hide resolved
website/integrations/services/pocketbase/index.mdx Outdated Show resolved Hide resolved
website/integrations/services/pocketbase/index.mdx Outdated Show resolved Hide resolved
website/integrations/services/pocketbase/index.mdx Outdated Show resolved Hide resolved
website/integrations/services/pocketbase/index.mdx Outdated Show resolved Hide resolved
website/integrations/services/pocketbase/index.mdx Outdated Show resolved Hide resolved
nicedevil007 and others added 11 commits February 1, 2025 22:38
Co-authored-by: dominic-r <[email protected]>
Signed-off-by: NiceDevil <[email protected]>
Co-authored-by: dominic-r <[email protected]>
Signed-off-by: NiceDevil <[email protected]>
Co-authored-by: dominic-r <[email protected]>
Signed-off-by: NiceDevil <[email protected]>
Co-authored-by: dominic-r <[email protected]>
Signed-off-by: NiceDevil <[email protected]>
Co-authored-by: dominic-r <[email protected]>
Signed-off-by: NiceDevil <[email protected]>
Co-authored-by: dominic-r <[email protected]>
Signed-off-by: NiceDevil <[email protected]>
Co-authored-by: dominic-r <[email protected]>
Signed-off-by: NiceDevil <[email protected]>
Co-authored-by: dominic-r <[email protected]>
Signed-off-by: NiceDevil <[email protected]>
Co-authored-by: dominic-r <[email protected]>
Signed-off-by: NiceDevil <[email protected]>
Co-authored-by: dominic-r <[email protected]>
Signed-off-by: NiceDevil <[email protected]>
Co-authored-by: dominic-r <[email protected]>
Signed-off-by: NiceDevil <[email protected]>
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.

2 participants