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

Add configuration flag to disable splitting keys on '.' #42

Merged
merged 2 commits into from
Mar 12, 2024

Conversation

jordan-powers
Copy link
Contributor

I recently set up an instance of Kanboard using this plugin to sign in users through Slack. As part of the set-up, I wanted to have users grouped by the id of the workspace associated with their slack account. The Slack userInfo response does include this information under the key https://slack.com/team_id, so I configured the "Groups Key" to be https://slack.com/team_id.

However, this created two issues:

  1. The key https://slack.com/team_id contains a '.', causing the GenericOAuth2UserProvider::getKey function to split it up. Essentially, the plugin thought that the key was referring to a member of a sub-object of the response, when it was actually just a literal '.' in the key.
  2. Once I fixed 1, I was still getting a crash. Turns out, the plugin expects the data returned from the "Groups Key" to be an array of groups. However, I was just getting a literal string, causing array_unique to throw an error.

My solution is the following:

  1. Add a configuration option to disable splitting keys. None of my keys are composite, so I don't need the splitting functionality. So I added the oauth2_split_keys setting, which will disable key splitting when set to '0'.
  2. Wrap the $groups in an array if it is not already an array. That way, if the "Groups Key" returns just a single group, it will be automatically wrapped into an array of length 1.

@fguillot fguillot merged commit 93ae46c into kanboard:master Mar 12, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants