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

Feat/mfa #8

Merged
merged 17 commits into from
Oct 11, 2023
Merged

Feat/mfa #8

merged 17 commits into from
Oct 11, 2023

Conversation

GoldenSheep402
Copy link
Contributor

先解决TOTP


// TOTP
message EnableTOTPRequest {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

enable的时候得让用户输入有效的TOTP 防止用户搞错了导致无法登陆

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

也就是说enable应该是一个连贯的过程

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

改了,一个AddTOTP,添加进去一个TOTP(假如已有未激活的覆盖掉,即只允许有一个not active的数据存在,假如已有active的则不允许操作),响应返回密钥,在数据库内表示为not active,一个ActiveTOTP,激活已导入的TOTP

string new_email = 1;
string old_email = 2;
string code_old_email = 3;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

切换主副邮箱为啥要验证码鸭

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

确实 好像不是很必要

proto/auth/v1/auth_service.proto Show resolved Hide resolved

// TOTP
// 添加TOTP
rpc AddTOTP(AddTOTPRequest) returns (AddTOTPResponse) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AddTOTP,把新生成的添加到数据库,标注为not active,然后返回密钥和恢复码,在ActiveTOP中提交一个Code,如果通过则设置为active,而且AddTOTP不允许在有active记录时添加而且只允许一个not active的存在,已有not active的则用新数据覆盖掉它

@asjdf
Copy link
Member

asjdf commented Oct 10, 2023

我感觉我们先捋一下思路?这里主要有邮箱验证和TFA验证两种。我们之所以要获取TFA的码是因为我们要确认用户安装/登记TFA的流程没有问题,能够通过服务器的验证。也就是说,我们不需要关心用户TFASecret的正确性,只需要保证前端那边传过来的用户输入的TFA码能对上TFASecret即可。实际上TFASecret在第一次绑定的时候可以由前端提供,因为这不影响绑定后的安全性。

而用户如果要走邮箱验证,直接走就好了,因为用户已经完成了邮箱的绑定,也就是说已经验证过邮箱的归属权了,感觉不太有必要再次验证了。用户可以选他所有已经绑定过的邮箱作为MFA邮箱,且这个选择应该是在用户需要MFA的时候自行选择。

我们所要做的应该是:

  1. 提供一个TFA的绑定机制
  2. 在进行MFA时给用户提供一个选择MFA渠道的方式
  3. 提供MFA渠道设置的方式。(比如是否使用邮箱/手机作为MFA渠道

@GoldenSheep402
Copy link
Contributor Author

1.绑定机制的话,对于TOTP我的思路是这样,Add的时候在数据库中存入未激活的数据(已经与用户绑定),然后Active的时候将其启用。然后Email确定所有权后可以添加到备用邮箱中,用户可以自己切换主邮箱。
2.在进行MFA时给用户提供一个选择MFA渠道的方式 :通过status获取到mfa状态,从这个里面去选。
3.提供MFA渠道设置的方式。 注册的时候的邮箱直接作为MFA的email渠道的主邮箱,添加TOTP需要通过email的认证。

@asjdf
Copy link
Member

asjdf commented Oct 10, 2023

hummm,我不是说这个机制不可行,主要想表达的意思是有没有必要这么做,比如针对你提出的第一点,用户在进行绑定的时候,我们只需要让前端提供TFAsecret和TFACode就好了。one shot即可完成。

然后就是邮箱的问题,邮箱绑定和启用邮箱2FA是两个分离的需求,启用邮箱2FA依赖于邮箱绑定。

第二点和我理解的一样

第三点和我理解的也差不多,我们不妨更抽象一点来看这个操作。我们之所以需要做email验证,实际上是在提升回话权限,也就是将当前会话提升至sudo模式。那么实际上可以拆分成两个步骤了,第一步,通过MFA临时提升账号权限;第二步,做TFA绑定

  1. 前端通过某些接口判断当前会话是否是sudo模式(sudo仅保持一小段时间,状态可通过redis存储
  2. 若不在sudo模式或sudo模式剩余时间过短,则弹框让用户完成MFA,使会话升级至sudo模式
  3. 用户进行高风险操作,如TFA绑定。(后端在高风险操作处理前还需要再次检查账户状态

@GoldenSheep402
Copy link
Contributor Author

这样,第一点我没考虑周全,第三点我加了个GetAccountStatus,返回sudo_ttl告知前端sudo剩余时间。

@asjdf
Copy link
Member

asjdf commented Oct 10, 2023

我晚上re

proto/auth/v1/auth_service.proto Outdated Show resolved Hide resolved
proto/auth/v1/auth_service.proto Show resolved Hide resolved
proto/auth/v1/auth_service.proto Show resolved Hide resolved
Copy link
Member

@asjdf asjdf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ci好像被误动到了

.github/workflows/check.yaml Outdated Show resolved Hide resolved
proto/auth/v1/auth_service.proto Show resolved Hide resolved
proto/auth/v1/auth_service.proto Show resolved Hide resolved
Copy link
Member

@asjdf asjdf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM ❤️

合分支用squash吧,中间commit好多

@GoldenSheep402 GoldenSheep402 merged commit a6990b0 into main Oct 11, 2023
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.

2 participants