-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
[WIP] Feature/split login strategies #1053
[WIP] Feature/split login strategies #1053
Conversation
…split-login-styles # Conflicts: # src/routes/login/Login.js
…split-login-strategies Conflicts: src/server.js
I would suggest you introduce a directory hierarchy, and split the various strategies to files. |
@advance512 |
…split-login-strategies Conflicts: src/server.js
…split-login-styles
…eature/split-login-strategies Conflicts: src/core/passport.js
@koistya I'm ready for review |
- repair subroute
…split-login-strategies-wip
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 is still very inspiring, but I'm sure there are better ways and more important missing code in auth handling..
buttonText: 'Log in with Facebook', | ||
}, | ||
google: { | ||
icon: 'M30 13h-4V9h-2v4h-4v2h4v4h2v-4h4m-15 2s-2-1.15-2-2c0 0-.5-1.828 1-3 ' + |
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'm really sure that this should be separate svg file
buttonText: 'Log in with google', | ||
}, | ||
twitter: { | ||
icon: 'M30 6.708c-1.105.49-2.756 1.143-4 1.292 1.273-.762 2.54-2.56 ' + |
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.
separate svg resource...
buttonText: 'Log in with twitter', | ||
}, | ||
github: { | ||
icon: 'M8 0C3.58 0 0 3.58 0 8c0 3.54 2.29 6.53 5.47 7.59.4.07.55-.17.55-.38 ' + |
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.
and this too...
// https://github.com/ | ||
github: { | ||
id: process.env.GITHUB_CONSUMER_KEY || 'xxxxxxxxxxxxxxxxxxxxx', | ||
secret: process.env.GITHUB_CONSUMER_SECRET || 'xyxyxyxyxyxyxyxyxyxyxyxyxyxyxyxyxyxyxyxyxy', |
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.
Very nice default strings 😄 — of course, not really. Something shorter which can be checked on runtime.. or omit default entirely can be nicer
import * as passportGoogle from './google'; | ||
import * as passportGithub from './github'; | ||
import * as passportTwitter from './twitter'; | ||
import * as passportCustom from '../../custom/passport'; |
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.
Can this be explicit?
|
||
const schema = new Schema({ | ||
query: new ObjectType({ | ||
name: 'Query', | ||
fields: { | ||
me, | ||
news, | ||
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.
Is this really needed? I think, can you easily implement near-infinity trusted possibilities of authentication strategies without touching code? I think, sending localization messages like I did is outer-limit, but sending authentication methods? This is really silly 😆
}, | ||
}); | ||
|
||
export default AuthType; |
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.
Most customizable CMS on the world 😄
@ClemensSahs thank you very much for this PR! Unfortunately, we have close it due to inactivity. Feel free to re-open it or join our Discord channel for discussion. NOTE: The |
Here is a first idea for splitting the auth strategies, feedback are welcome ;)
future steps:
add more strategies (logic)
best regards