Allow setting a custom session name #46
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pbek
reviewed
Mar 7, 2023
| @@ -152,10 +152,17 @@ public function createToken() | |||
| throw new OCSForbiddenException(); | |||
| } | |||
|
|
|||
| $name = $this->request->getParam('session-name'); | |||
|
|
|||
| if ($name === '') { | |||
There was a problem hiding this comment.
Isn't the name null when the param isn't set?
There was a problem hiding this comment.
Might be, like I said I haven't tested this, I will take a closer look tomorrow. Just an idea of what I mean for now.
pbek
reviewed
Mar 7, 2023
|
|
||
| public function getAllowCustomName(): bool | ||
| { | ||
| return $this->config->getAppValue('webapppassword', 'allowCustomName') == 'true'; |
There was a problem hiding this comment.
Can you please use something like token_name_suffix instead of allowCustomName.
pbek
reviewed
Mar 7, 2023
| if ($name === '') { | ||
| $name = $targetOrigin.' '.$this->request->getHeader('USER_AGENT'); | ||
| } else { | ||
| 'App Password: '.$name; |
There was a problem hiding this comment.
This line doesn't do anything. 😉
And can you please set the previous name by default.
pbek
reviewed
Mar 7, 2023
| return $this->config->getAppValue('webapppassword', 'allowCustomName') == 'true'; | ||
| } | ||
|
|
||
| public function setAllowCustomName($value) |
There was a problem hiding this comment.
Can you please use something like setTokenNameSuffix.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
An idea of what I mean in #45
This is a draft as I have not tested it, I don't see why it wouldn't work, but just to be sure.
Thid PR includes adding a way for a client to indicate how their should be named. This behavior is only enabled if the
allowCustomNameconfig is set totrue. I did not add to the configuration page so the config would have to be set in theconfig.phpfile.