Skip to content

feat: OAuth trailing slash variable#1163

Merged
JohnVillalovos merged 1 commit intoLibreBooking:developfrom
PVTGoesen:OAuth2-trailing-slash
Mar 23, 2026
Merged

feat: OAuth trailing slash variable#1163
JohnVillalovos merged 1 commit intoLibreBooking:developfrom
PVTGoesen:OAuth2-trailing-slash

Conversation

@PVTGoesen
Copy link
Copy Markdown
Contributor

Some OAuth2 provider need a trailing slash in the authorization URL. This changes let you choose if you want to cut or keep the trailing slash in oauth2 authorize URL.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a new OAuth2 configuration option to control whether a trailing slash on the configured authorize URL is preserved or trimmed when generating the login redirect URL.

Changes:

  • Introduces authentication.oauth2.trailing.slash (boolean) config key with default false.
  • Updates OAuth2 authorize URL construction in LoginPresenter::GetOauth2Url() to conditionally rtrim() the URL.
  • Documents the new setting in config/config.dist.php.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
Presenters/LoginPresenter.php Uses the new config flag to decide whether to trim a trailing slash from the OAuth2 authorize URL.
lib/Config/ConfigKeys.php Defines the new AUTHENTICATION_OAUTH2_TRAILING_SLASH config key metadata.
config/config.dist.php Adds the default oauth2.trailing.slash setting with an explanatory comment.

You can also share your feedback on Copilot code review. Take the survey.

Comment thread Presenters/LoginPresenter.php Outdated
Comment thread Presenters/LoginPresenter.php Outdated
Copy link
Copy Markdown
Collaborator

@JohnVillalovos JohnVillalovos left a comment

Choose a reason for hiding this comment

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

Thanks @PVTGoesen

Looks good except for the name of the config option.

Comment thread Presenters/LoginPresenter.php Outdated
Comment thread lib/Config/ConfigKeys.php Outdated
@JohnVillalovos
Copy link
Copy Markdown
Collaborator

@PVTGoesen Any updates?

Comment thread config/config.dist.php Outdated
Comment thread lib/Config/ConfigKeys.php Outdated
Copy link
Copy Markdown
Collaborator

@JohnVillalovos JohnVillalovos left a comment

Choose a reason for hiding this comment

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

Thanks @PVTGoesen

LGTM

@JohnVillalovos JohnVillalovos force-pushed the OAuth2-trailing-slash branch 2 times, most recently from 7bf2c2c to 8948a77 Compare March 23, 2026 03:39
Copy link
Copy Markdown
Collaborator

@JohnVillalovos JohnVillalovos left a comment

Choose a reason for hiding this comment

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

Thanks @PVTGoesen

LGTM

Adds a variable for OAuth, to keep the configured authorize URL's
trailing slash

Closes: LibreBooking#917
@JohnVillalovos JohnVillalovos force-pushed the OAuth2-trailing-slash branch from ec56c76 to 8e72ea2 Compare March 23, 2026 03:49
@JohnVillalovos JohnVillalovos enabled auto-merge (rebase) March 23, 2026 03:50
@JohnVillalovos JohnVillalovos merged commit 662fa90 into LibreBooking:develop Mar 23, 2026
16 checks passed
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.

3 participants