-
Notifications
You must be signed in to change notification settings - Fork 24
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
App owned logins #330
App owned logins #330
Conversation
00baad3
to
f4cb93f
Compare
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.
import { USERNAME_STORAGE_KEY } from "../common/index.js" | ||
|
||
export const init = async (): Promise<State | null> => { | ||
console.log("initialize local auth") |
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.
We could switch out console.log
with the debug logging functions, so they only print with setup.debug({ enabled: true })
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.
isUsernameValid: (username: string) => Promise<boolean> | ||
isUsernameAvailable: (username: string) => Promise<boolean> |
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.
Just a Q for clarification: Why do we abstract these functions? We're using the same implementation for both local auth as well as auth-lobby based auth it seems.
Is this so that someone would in principle be able to implement their own "username database"? If so, is that possible today? I think we'd need email-like identifiers for that, i.e. something like matheus23@fission.codes
usernames instead of matheus23
. E.g. in the DNSLink fetching code.
Maybe it doesn't hurt having these abstractions in here today already, but I'm not sure. 🤔
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.
Yes, this is so that you can plug in any arbitrary lookup method, rather than forcing everyone to make it look like Fission's. For example, web3.storage has this in a big database table that's at a different endpoint.
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.
RE validation: services may have special validation rules for their service (e.g. no curse words in Spanish). I believe that the plan is to provide defaults and let users override them if needed.
something like matheus23@fission.codes usernames instead of matheus23
That's more of an interoperability question. Blaine calls this style the "Jesus of Nazareth" format, and I agree that it reads very well (in English at least)
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.
(But per service, you can imply the host, in the same way that Discord hides the nonce in your username unless there's a collision on that server)
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.
This PR introduces dependency injection for initialization and account registration (along with the various username validation features). This allows apps to override these and potentially use alternative authentication backends / mechanisms.
It also introduces a "local" version that allows an app to init/register accounts in a self-contained manner like this:
Closes #316
Closes #302