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

New authentication service for semantic.works written in javascript #1

Merged
merged 29 commits into from
Oct 10, 2021

Conversation

aatauil
Copy link
Owner

@aatauil aatauil commented Aug 1, 2021

New authentication service, merging the functionality of the existing login-service & registration-service. Not to reinvent the wheel but rather to be a clone but instead written in javascript. Some small stuff is different though like using 'email' instead of 'nickname' but queries are almost a one-one copy.

Readme should clarify a lot.

Some comments

  • Would actually like to see this repo being migrated to the mu-semtech github org
  • Delete account is not functional yet, dont know the best approach -> should everything related to the account be deleted oradd a triple that marks the account as 'deleted' and leaving all data?
  • Better error handling is needed, perhaps with unique id for easy debugging
  • This is meant as a quick way to have authentication. A "Template" that the user can clone and extend its functionality as needed instead of providing a extensive service full of functionality the user might never use
  • Will write some guides in wiki that will show how to extend service with 'forgotten password', '2fA', 'email notification', ... functionality in the future

@madnificent
Copy link

This is cool!

The PR currently writes the accounts information to a single graph. In may settings, this graph would be determined by the some information related to the user (most often their uuid, but it could also be a related organization, the id of their account, ...).

Perhaps we should create an async function to yield the graph? Perhaps some sensible defaults can be supplied through configuration.

Cfr the hashing: I see a configurable salt, which is good. This ensures a generic rainbow-table cannot be used. I would add a per-password unique string (stored int he database) as an extra bit of random data to the password. This would ensure a rainbow-table couldn't be constructed against the whole database.

@madnificent
Copy link

Thinking about this further, the data will likely need to be split across multiple graphs. Some of this could be handled by mu-authorization but there is a bit of a chicken-and-egg problem in that mu-authorization doesn't know what data will be present before trying to insert that exact data. Either we should allow specific triples to be written to specific graphs, or we should execute some tricks to inform mu-authorization ahead-of-time.

Tricking mu-authorization would involve removing the mu-auth-allowed-groups, storing the login information into a temporary graph using mu-auth-sudo, and then sending the data to mu-authorization again without mu-auth-allowed-groups. At that point mu-authorizition can use the temporary graph to figure out who the user is. Once that's done, we can remove the specific data we added to the temporary graph.

It needs further thought (and perhaps experimentation?) to figure out the most sensible approach.

I've enjoyed playing with this service. It's like a tasty 🍪 and I'm looking forward to gradually improve the recipe even further.

This is probably more like a personal preference, I like it more since I find that try/catch blocks add noise, the use function in app.js takes care of catching errors instead
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.

None yet

2 participants