app icon indicating copy to clipboard operation
app copied to clipboard

Added TOTP-like alias generation and included some extra information on CONTRIBUTING.md

Open zafuru opened this issue 2 years ago β€’ 14 comments

I like having short aliases, but UUIDs are very long, and I prefer using random characters instead of two random words, so I constantly find myself creating custom aliases like:

[email protected]

This pull request adds a mode that allows creating random aliases just like that one, which is very similar to the code we might get when using one-time-passwords.

I also modified CONTRIBUTING.md and explained that to copy the example.env, we first have to go back to the root folder. I spent a minute trying to find out why I could not copy that file πŸ˜…

Please do let me know if there is anything else I need to do before this can be merged 😊

zafuru avatar May 17 '22 04:05 zafuru

Hey, thanks for the PR! It seems to be a nice feature. I'll leave some comments:

  • The keyspace is too short. If we get to a million aliases we will have a 1 in 2k chance of collision. Since we can't use upper and lower case characters, at least make it 9 chars long.
  • Please don't call it totp. It stands for time-based one time password. It's neither time based nor a password. Can we call it short or something like that?
  • Can you add a test to make sure the alias is generated as expected?

acasajus avatar May 17 '22 07:05 acasajus

  • Please don't call it totp. It stands for time-based one time password. It's neither time based nor a password. Can we call it short or something like that?

I'd say short_uuid to give a clearer context

ntnhon avatar May 17 '22 07:05 ntnhon

Hey, thanks for the PR! It seems to be a nice feature. I'll leave some comments:

* The keyspace is too short. If we get to a million aliases we will have a 1 in 2k chance of collision. Since we can't use upper and lower case characters, at least make it 9 chars long.

* Please don't call it totp. It stands for time-based one time password. It's neither time based nor a password. Can we call it `short` or something like that?

* Can you add a test to make sure the alias is generated as expected?

@acasajus You're right, I'll increase the keyspace, but can we make this customizable when using custom domains? Maybe not in this PR as it will require adding a button to the front-end, too. I know it's not a TOTP, but I'm very bad at naming and that's the first thing that came to mind when I saw word and uuid πŸ˜… and yeah, I'll add a test case.

  • Please don't call it totp. It stands for time-based one time password. It's neither time based nor a password. Can we call it short or something like that?

I'd say short_uuid to give a clearer context

@ntnhon That does sound But that can also be misleading because it would not be a uuid, maybe alphanumeric? slid? How about short_code?

zafuru avatar May 17 '22 15:05 zafuru

I'm glad you told me to write some tests because the previous code had some bugs, but now it should be better. I ended up calling this alias generator random_string like the suffix generator

zafuru avatar May 17 '22 17:05 zafuru

Funny, I was thinking that it'd be great if we had a dictionary in the enum class for that, to avoid modifying the same code in the future, but I didn't know that we could already do that. Pretty cool. Will update the code soon 😊

zafuru avatar May 26 '22 02:05 zafuru

@acasajus Done! I think. I also worked a bit on the base enum class

zafuru avatar May 26 '22 18:05 zafuru

This would be lite. I would love to see this be added.

c0nfigurati0n avatar Aug 04 '22 12:08 c0nfigurati0n

Looks good!

However I think "random characters" is a better name than "random string", technically all current email generating methods are "random string" methods. Just a small input though.

Additionally I think random character alias length should be defined in the environment variables, just like how the path to the words file is. This to easily propagate eventual changes to the alias length across the whole code base.

kelszo avatar Aug 11 '22 20:08 kelszo

Is there any update on when this could be implemented? ☺️

EDIT: Related issue here #1252

Nautman avatar Oct 16 '22 15:10 Nautman

@Nautman it seems that there are still some questions that are not answered/addressed yet in the PR.

nguyenkims avatar Oct 18 '22 13:10 nguyenkims

@zafuru anything new to this? I am really hoping that this could be merged soon! thanks for your work!

leon1995 avatar Nov 16 '22 19:11 leon1995

Still no update to this?

Nypheena avatar May 08 '23 00:05 Nypheena

I personally like this feature, hope this can be merged soon. For reference, DuckDuckGo Email Protection currently uses 8 random lowercase letters and numbers for their address generation and it seems to work well for them.

developStorm avatar May 11 '23 13:05 developStorm

Agree. It will be good if implemented

ghost avatar Nov 06 '23 08:11 ghost