-
Notifications
You must be signed in to change notification settings - Fork 737
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
feat: 增加 egg-passport-local example #54
base: master
Are you sure you want to change the base?
Conversation
passport/app.js
Outdated
module.exports = app => { | ||
app.passport.verify(async (ctx, user) => { | ||
user.photo = user.photo || 'https://zos.alipayobjects.com/rmsportal/JFKAMfmPehWfhBPdCjrw.svg'; | ||
user.id = user.provider || 'local'; |
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.
user.id 这个取值不对吧
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.
我的想法是为了跟其他 strategy 的展示结果对应
// home.js
Logined user: <img src="${ctx.user.photo}"> ${ctx.user.displayName} / ${ctx.user.id} | <a href="/logout">Logout</a>
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.
user.id
的语义应该是用户唯一 ID,不应该是你这个 local 的取值吧
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.
egg-passport-local
只有 username,password,provider,user.id
得由应用方产生
// app.js
module.exports = app => {
app.passport.verify(async (ctx, user) => {
const existsUser = await ctx.model.User.findOne({ id: user.id });
if (existsUser) {
return existsUser;
}
const newUser = await ctx.service.user.register(user); // 生成 id
return newUser;
});
};
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.
是的,所以你这里是没必要写这个 ID 的。
即使要显示 strategy,那应该是 ctx.user.provider 才对
passport/app/controller/home.js
Outdated
ctx.body = ctx.user; | ||
} else { | ||
ctx.body = ` | ||
<h1>egg-passport-local login page</h1> |
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.
改为 egg-view-nunjucks ?
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.
OK,我改下
|
||
app.passport.mount('weibo'); | ||
app.passport.mount('github'); | ||
app.passport.mount('bitbucket'); | ||
app.passport.mount('twitter'); | ||
const localStrategy = app.passport.authenticate('local'); |
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.
想起来了,之前我 app.passport.mount('twitter');
的想法是会去判断对应的 strategy 如果提供了 mount 方法就用它的,否则用默认的。不过这样先也行吧
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.
你的想法是不是这样:
可以由各个 strategy 实现自己的 mount 方法,用户调用 mount 的时候优先调用 strategy 的 mount,没有的的话才调用 egg-passport
默认的 mount
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.
嗯
passport/config/config.default.js
Outdated
@@ -20,3 +20,10 @@ exports.passportTwitter = { | |||
key: 'g', | |||
secret: 'h', | |||
}; | |||
|
|||
// 为了演示方便这里把 csrf 暂时关闭 |
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.
英文或干掉,其实用 egg-view-nunjucks 后就不用关了。
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.
OK
passport/app.js
Outdated
@@ -2,7 +2,7 @@ | |||
module.exports = app => { | |||
app.passport.verify(async (ctx, user) => { | |||
user.photo = user.photo || 'https://zos.alipayobjects.com/rmsportal/JFKAMfmPehWfhBPdCjrw.svg'; | |||
user.id = user.provider || 'local'; | |||
user.id = user.provider; |
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.
不是,我的意思是,为什么你要把 user.id
赋值为 user.provider
?
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.
搞错
egg-passport-local 那边还没发布吧? 那个 PR 要接手搞完 |
好的 |
rebase + ci |
Checklist
npm test
passesAffected core subsystem(s)
Description of change
增加
egg-passport-local
example