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

Forced Discord Account and Mail Verification #93

Closed
wants to merge 1 commit into from
Closed

Forced Discord Account and Mail Verification #93

wants to merge 1 commit into from

Conversation

LogischJo
Copy link
Contributor

@LogischJo LogischJo commented Jun 25, 2021

#72 This is configurable using:
VERIFIED_DISCORD_TO_MAKE_SERVER and VERIFIED_EMAIL_TO_MAKE_SERVER

#72 This is configurable using:
VERIFIED_DISCORD_TO_MAKE_SERVER and VERIFIED_EMAIL_TO_MAKE_SERVER')
@@ -38,6 +33,16 @@ public function create()
return redirect()->route('servers.index')->with('error', "You've already reached your server limit!");
}

//Required Verification for creating an server
if(Configuration::getValueByKey('VERIFIED_EMAIL_TO_MAKE_SERVER') === 'true' && !Auth::user()->hasVerifiedEmail()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

You can add a default value to 'getValueByKey' by using the second param ('KEY' , 'DEFAULT_VALUE') that way if the key is somehow missing this would still work and not trow errors or unexpected behaviour :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in new PR (#95) (sadly I was not able to commit into this one)

}

//Required Verification for creating an server
if(Configuration::getValueByKey('VERIFIED_DISCORD_TO_MAKE_SERVER') === 'true' && !Auth::user()->discordUser) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

check comment above :)
To what method have you added this script? should be added to both the create and store method to prevent any bypasses

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in new PR (#95) (sadly I was not able to commit into this one)

Configuration::firstOrCreate([
'key' => 'VERIFIED_DISCORD_TO_MAKE_SERVER',
], [
'value' => 'true',
Copy link
Collaborator

Choose a reason for hiding this comment

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

not much besides turning on the extra spacing options for array's :P
could you also rename the keys to something like (FORCE_EMAIL_VERIFICATION)
and perhaps add a small description explaining what this option does :)
I've already done this to all of the config options, but it seems like you are running an older version.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in new PR (#95) (sadly I was not able to commit into this one)

@AVMG20
Copy link
Collaborator

AVMG20 commented Jun 26, 2021

Already solved

@AVMG20 AVMG20 closed this Jun 26, 2021
1day2die added a commit that referenced this pull request May 3, 2023
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.

2 participants