wiki icon indicating copy to clipboard operation
wiki copied to clipboard

Updated Discord Authentication Module to allow optionally filtering based on Server Roles.

Open TomDakan opened this issue 2 years ago • 16 comments

Updated Discord Authentication Module to allow optionally filtering based on Server Roles. Added custom authentication error message. Added Discord-Oauth2 to package.json. Discord-Oauth2 is used to fetch role data from the specified server in module/discord/authentication.js

TomDakan avatar Apr 05 '23 19:04 TomDakan

Still getting familiar with vs dev containers and working with git. Discovered that some of my changes were reverted at some point, so I need to recover them and retest.

TomDakan avatar Apr 07 '23 15:04 TomDakan

Hey @TomDakan, thank you for opening this PR! ❤️ If you're still interested in this change, are you planning to finish this draft? I'm willing to use this feature and I'm interested in finishing those changes if it's not possible for you to finish it at the moment 🙏

tboba avatar Sep 25 '23 17:09 tboba

@tboba hey, i've been using it for the last few months and it's been working fine, but I had to do a few things that wouldn't be best practice for the actual PR to get the dependencies included that I needed. I'll try to look at my repo in the next few days and see if I can clean it up to produce a finalized PR.

TomDakan avatar Sep 25 '23 18:09 TomDakan

@TomDakan sure! If you would like to I can do a code review for you in about an hour 👍

Also, how are you using this change right now? Are you making your own patch and building Wiki.js from source with it, or doing it in other way? I'm just asking, because I can't find any guide how to build Wiki.js from source 🥲

tboba avatar Sep 25 '23 18:09 tboba

@tboba the "simplest" way I found to do it was to create another volume and bind it into the authentication module directory as a new custom module. I remember trying to figure out how to build a fresh production droplet with my changes to the generic discord auth module, and not being able to figure it out in a reasonable amount of time. I found the dev environment very complicated. There is info on how to build a production build at the bottom of this article: https://docs.requarks.io/dev that you might find useful if you haven't read it yet.

So your docker command would be something like: docker run -e LETSENCRYPT_DOMAIN=wiki.domainname.com -e [email protected] -e SSL_ACTIVE=1 -e DB_TYPE=postgres -e DB_HOST=db -e DB_PORT=5432 -e DB_PASS_FILE=/etc/wiki/.db-secret -v /etc/wiki/.db-secret:/etc/wiki/.db-secret:ro -e DB_USER=wiki -e DB_NAME=wiki -e UPGRADE_COMPANION=1 --restart=unless-stopped -h wiki --mount type=bind,source=/mnt/volume_sfo3_01/discord_custom,target=/wiki/server/modules/authentication/discordCustom --network=wikinet -p 80:3000 -p 443:3443 ghcr.io/requarks/wiki:2

TomDakan avatar Sep 25 '23 18:09 TomDakan

Also, I think all of my changes are committed now. I don't have time to test it today, but hopefully it will get you going.

TomDakan avatar Sep 25 '23 18:09 TomDakan

Thanks! 😄 Also, let me leave a couple of comments about proposed changes, so it could land on the main successfully ✌️

tboba avatar Sep 25 '23 19:09 tboba

@tboba also, I think I got all your changes integrated correctly. Let me know if i screwed something up. Particularly moving the intitialization of the DiscorOauth2 object outside of the initialization of the passport strategy.

TomDakan avatar Sep 25 '23 22:09 TomDakan

Also, thanks for the review. I don't spend much time working in javascript.

TomDakan avatar Sep 25 '23 22:09 TomDakan

Thanks for completing this draft @TomDakan ❤️ @NGPixel can you please take a look on those changes in your free time? 🙏

tboba avatar Sep 26 '23 22:09 tboba

You shouldn't have a package.json in the discord module folder, as you're submitting a PR to make it part of the official repo. The per-module package.json is only for custom modules that are not part of Wiki.js itself.

NGPixel avatar Sep 26 '23 23:09 NGPixel

@NGPixel ok, I'll remove it. Does that mean I should modify the main package.json to include the dependency? Nevermind, I forgot that I already had.

TomDakan avatar Sep 26 '23 23:09 TomDakan