Map Discord roles to NextCloud groups #395
Conversation
|
Currently this only reads Discord role IDs, not their names, which makes them very verbose in the UI:
Obviously I would like it to read their names instead. For that, I believe it needs to call [GET/guilds/{guild.id}/roles, but for that I fear that it probably requires a. creating a Bot User, b. inviting that Bot User to the target guilds, c. giving the Bot User's token to this plugin d. using the Bot User's token to call that API, all of which is quite a bit more complicated than the status quo. It might be good enough to just impose a group mapping like @pktiuk suggested; it'll be annoying for the admin to set it up the first time and every time they add a new role ((I have no idea how to find out the role IDs from the Discord UI, they just don't appear anywhere)), but it'll be stable at least. Currently this reads ALL roles from ALL guilds someone is in. It should probably be restricted to just the guilds in the $allowedGuilds list. Also, I'm not sure if roles have globally unique IDs across Discord or not? What happens if there's an ID collision between two guilds? Do we need to prefix the roles with the guild ID to make sure they're unique? That sounds...even worse. I think, also, this patch belongs better in HybridAuth. But I tried patching the vendored copy of it under 3rdparty and couldn't get it to work, until I learned from ef5e24c that lib/Service/ProviderService.php overrides a lot of its settings so that patching it wouldn't work anyway. I still think that's a good idea, but it will require some coordination and deeper thought -- and like, say, not everyone is going to want to use it, which means the |
|
I've got group mapping protoyped. But I didn't take the time to build a whole UI for it, so so far all I have is this patch, which I'm not going to attach to the branch because it's too specific to my use case, but I want to record it here anyway because it shows what needs doing: diff --git a/lib/Service/ProviderService.php b/lib/Service/ProviderService.php
index 5e737f3..011332e 100644
--- a/lib/Service/ProviderService.php
+++ b/lib/Service/ProviderService.php
@@ -423,6 +423,15 @@ class ProviderService
// TODO: /member returns roles as their ID; to get their name requires an extra API call
// (and perhaps extra permissions?)
}
+
+ // HACK: because there's no UI for mapped groups under the Discord connector like there is under OIDC or Discourse
+ // code in our mapping here
+ $profile->data['group_mapping'] = array(
+ '5916019825232292399' => "admin",
+ '3908310574390159257' => "Stars",
+ '1647820633687053577' => "Coordinators",
+
+ );
}
if (!empty($config['logout_url'])) {This works! I can assign/unassign people roles in Discord, and the next time they log in (which I can force with NextCloud's "Wipe all devices" option in the /settings/users page) their groups will be updated. And with "Restrict login for users without mapped groups" turned on this means that only the roles listed there are allowed to log in, giving me some peace of mind. Some important points:
|
Yep, mapping assumes "exact foreign group -> exact NC group".
Just disable
There is |
|
@kousu |
|
@pktiuk I did not add any useful information (almost), soo ) |
This is useful! And obvious in retrospect but I missed it. I assumed mapped groups would count as their target names. |
There's no code changes to make from this yet 😵💫 I haven't added UI to set up the group mappings yet. For my team, I edited |
|
Looking ahead, I'm stumped on how to do the workflow for adding new mapped groups. With "Custom Discourse" the UI looks like this empty free-form box:
Which is maybe fine for Discourse where the group names are obvious, but with Discord the group names are ID numbers, and as far as I can, the only way to get those numbers are to use the API: The former works with the plugin's current basic OAuth2 setup, but the latter, I think, requires additionally creating a bot user and inviting it to each guild you might want to use it with. Which just seems like it greatly complicates the setup process. Currently I have been relying on |
|
When will the feature be integrated into the official version? |
|
Yeah, I want this too |
Unfortunately there's no timeline :( . It has no UI yet and what I do have written needs a lot more testing. If you want to speed it along, cheer me on, help me test, and if you want to learn a bit of PHP or Angular, read https://nextcloud.com/developer/ and give me a hand getting the code over the finish line. |
no, I confused myself; this PR is about groups and their names, not users and their names. And mapping role IDs to named groups ( #395 (comment) ) is a 99% better solution than trying to put strings from the Discord API into NextCloud. |
I would help test too if you need. I need this for my project |
angular sucks, learn vue 😄 |
@FabiChan99 taught me something! Discord has a Developer Mode under User -> Settings -> Advanced
and with this enabled, you can get your Role IDs out by right clicking on them under Server -> Settings -> Roles
That also gives me a good idea to at least get this to a usable state: the "Custom OIDC" and "Custom OAuth2" backends have this "Add Group Mapping" button which expects you to know the OIDC/OAuth2 Group ID strings:
If I can teach people how to turn on Discord Developer Mode then I can copy this UI and have them paste in the Role IDs the same way. 🎉 Once I get that working I'll undraft this and ask it to be merged. I would really like to have a button that pulls the Discord roles from their API, but that requires bouncing through their OAuth flow so it's more complicated than I want to think about right now. That can be a follow up. |
|
I've got it working! @SDS1234, @FabiChan99, @pktiuk, please test and let me know what you think. It shouldn't break anything -- it should be safe to switch between this one and the official release at any time. TestingTo try this out (from scratch, to give you a complete picture):
To switch back to the official release:
|
kousu
changed the title
kousu
force-pushed
the
discord-roles
branch
2 times, most recently
from
519640c
to
eb0ea74
Compare
| @@ -79,6 +79,7 @@ class ProviderService | |||
| 'id' => 'appid', | |||
| 'secret' => 'secret', | |||
| ], | |||
| 'group_mapping' => 'groupMapping', | |||
There was a problem hiding this comment.
I don't understand what $configMapping is about. We control the schema, can't we just force all the providers to use the same one?
There was a problem hiding this comment.
It just works as it should. I think with the little Tutorial, everyone can set this up!
There was a problem hiding this comment.
This still needs:
- Massage the "Copy Role ID" install instructions into README.md
I especially struggled with the distinction between "custom" auth providers
nextcloud-social-login/src/components/settings/provider-types.js
Lines 4 to 229 in 74def11
which render at the top of the settings page:
Custom Provider UI
and can have many instances of themselves,
and "providers" like Discord.com, which only have one of each and have custom code to support their proprietary APIs scattered throughout the plugin
nextcloud-social-login/lib/Settings/AdminSettings.php
Lines 41 to 51 in 74def11
Provider UI
They are both stored in the database as JSON, but in separate objects with similar but subtly different schemas -- particularly, because customs have to be nested in an Array because there can be many of them. This split causes a lot of special cases in the code. I got it working but I wonder if it could be more elegant.
| @@ -120,6 +120,20 @@ | |||
| <input type="text" :name="'providers['+name+'][guilds]'" v-model="provider.guilds"/> | |||
| </label> | |||
| </template> | |||
| <template v-if="provider.hasGroupMapping"> | |||
| <button class="group-mapping-add" type="button" @click="provider.groupMapping.push({foreign: '', local: ''})"> | |||
There was a problem hiding this comment.
For some reason the existence of the Add Group Mapping button widens the Discord box and breaks the layout. I do not understand. I've already spent 40 minutes looking at it.
Seriously what is this BS
Fix the CSS here.
| var groupMappingArr = [] | ||
| var groupMapping = data.providers[provType].groupMapping | ||
| if (groupMapping) { | ||
| for (var foreignGroup in groupMapping) { | ||
| groupMappingArr.push({foreign: foreignGroup, local: groupMapping[foreignGroup]}) |
| <template v-if="provider.hasGroupMapping"> | ||
| <button class="group-mapping-add" type="button" @click="provider.groupMapping.push({foreign: '', local: ''})"> | ||
| {{ t(appName, 'Add group mapping') }} | ||
| </button> | ||
| <div v-for="(mapping, mappingIdx) in provider.groupMapping" :key="mapping"> | ||
| <input type="text" class="foreign-group" v-model="mapping.foreign" /> | ||
| <select class="local-group" :name="mapping.foreign ? 'providers['+name+'][groupMapping]['+mapping.foreign+']' : ''"> | ||
| <option v-for="group in groups" :key="group" :value="group" :selected="mapping.local === group"> | ||
| {{ group }} | ||
| </option> | ||
| </select> | ||
| <span class="group-mapping-remove" @click="provider.groupMapping.splice(mappingIdx, 1)">x</span> | ||
| </div> | ||
| </template> |
| @@ -422,6 +441,7 @@ private function auth($class, array $config, $provider, $providerType = null) | |||
| } | |||
|
|
|||
| $profile->data['default_group'] = $config['default_group']; | |||
| $profile->data['group_mapping'] = $config['group_mapping']; | |||
There was a problem hiding this comment.
Does this need to be wrapped in an isset()? My PHP is kind of weak.
|
I'll take a look later (and maybe make some refactoring). In vacations now 😃 |
|
Implemented in eb1ab88 |
|
I have one question linked with implementation of this functionality. |
They're updated every login. There's a setting to tune it, but by default it does what you want:
|
|
I will update today and test it with my organisation |
|
I updated my own today finally (replacing the one from my git branch with the one from the app store) and it's looking good!! Thanks for fixing the CSS on that button @zorn-v.
I also appreciate that the default providers are now all hidden by default unless in use!! |
Fixes #390