-
Notifications
You must be signed in to change notification settings - Fork 535
#1119 email show permission and privacy improvements #1312
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
Conversation
This reverts commit cb60d7b
|
I've figured out that these changes cb60d7b were useless :') Sorry for the spam... |
| @@ -1,5 +1,5 @@ | |||
| <!-- AUTO-GENERATED FILE! Do not edit this directly --> | |||
| <!-- File auto-generated on Sat Apr 29 18:27:38 CEST 2017. See docs/commands/commands.tpl.md --> | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please update permissions.md as well :)
| int sid = frag[0].length() / 3 + 1; //Define the id view | ||
| int sdomain = frag[1].length() / 3 + 1; //Define the domain view | ||
| String id = frag[0].substring(0, sid) + "*****"; //Build the id | ||
| String domain = "***" + frag[1].substring(sdomain); //Build the domain |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I love the +1 as to guarantee that there will always be a letter visible.
Is there a reason for the asterisks in id vs. domain to be of different length? I might find it less confusing if my email test@example.org became something like te***@***le.org rather than te*****@***le.org.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is not a particular reason, so I'll adopt your solution ;)
| PlayerAuth auth = playerCache.getAuth(player.getName()); | ||
| if (auth != null && !Utils.isEmailEmpty(auth.getEmail())) { | ||
| commonService.send(player, MessageKey.EMAIL_SHOW, auth.getEmail()); | ||
| commonService.send(player, MessageKey.EMAIL_SHOW, emailMask(auth.getEmail())); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be good to make it configurable whether or not the email gets masked. It's not a must for now but I'm willing to bet that someone will create an issue for it in the future.
| */ | ||
| public class ShowEmailCommand extends PlayerCommand { | ||
|
|
||
| @Inject |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can use CommonService#getProperty ;) CommonService just offers the most used stuff from Settings, Messages and PermissionsManager to reduce the number of injected services.
| newProperty("Security.emailRecovery.cooldown", 60); | ||
|
|
||
| @Comment({ | ||
| "The maill showed using /email show will be partially hidden", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The mail shown using [...]
| " hidden email: mye***@***in.com" | ||
| }) | ||
| public static final Property<Boolean> EMAIL_PRIVACY = | ||
| newProperty("Security.privacy.email", false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was going to suggest to name it kind of like a question since it's a boolean, but then I started thinking of propositions (MASK_EMAIL, USE_EMAIL_MASKING or HIDE_FULL_EMAIL) and they don't really seem much better xP
|
I'm just nitpicking at this point—given @sgdc3's approval and mine feel free to merge this at any time. |
|
Just fix the wrong message and the service import optimization and we are ready to go! ;) |
|
Ok, let's merge this ;) |
* AuthMe#1119 new permission and email hider * Updated commands.md * Improved email hiding method * Revert "Improved email hiding method" This reverts commit cb60d7b * New config option, updated tests, config.md and permission_nodes.md * Moved to service import, fixed typo and updated config.md * Removed unused imports O.o
Now the command
/email showrequires the permission:"authme.player.email.see"and will return a partially hidden email.E.g.
normal:
my.email@example.comhidden:
my.*****@***ple.comformat:
1/3(id lenght) + *** + @ + *** + 2/3(domain lenght)