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

fix: correct device information update during account switch #6483

Merged
merged 1 commit into from
Aug 22, 2024

Conversation

guqing
Copy link
Member

@guqing guqing commented Aug 20, 2024

What type of PR is this?

/kind bug
/area core
/milestone 2.19.x

What this PR does / why we need it:

修复切换账号登录时设备信息更新不正确的问题

原因:

  1. 使用 admin 账号登录,此时会记录 device_id 的 cookie
  2. 退出登录,device_id 会保留在 cookie 中并随着新账号带到服务端
  3. 服务端根据 device_id 查询当前设备是否有对应的记录,但是没有校验用户名是否与当前登陆的一致然后就去更新登录时间
  4. 正确的处理是校验 device_id 是否有与之对应的记录并且用户名相同,如果不相同则认为是新设备重新生成 device_id

how to test it?

  1. 先清理 cookie 然后使用一个账号登录
  2. 退出登陆并切换新账号登录
  3. 检查新登录的账号的设备信息是否正确

Does this PR introduce a user-facing change?

修复切换账号登录时设备信息更新不正确的问题

@f2c-ci-robot f2c-ci-robot bot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Aug 20, 2024
@f2c-ci-robot f2c-ci-robot bot added this to the 2.19.x milestone Aug 20, 2024
@f2c-ci-robot f2c-ci-robot bot added the kind/bug Categorizes issue or PR as related to a bug. label Aug 20, 2024
@f2c-ci-robot f2c-ci-robot bot requested review from JohnNiang and LIlGG August 20, 2024 04:09
@f2c-ci-robot f2c-ci-robot bot added the area/core Issues or PRs related to the Halo Core label Aug 20, 2024
Copy link

Copy link

codecov bot commented Aug 20, 2024

Codecov Report

Attention: Patch coverage is 0% with 23 lines in your changes missing coverage. Please review.

Project coverage is 58.22%. Comparing base (bc10336) to head (9255cfc).
Report is 128 commits behind head on main.

Files Patch % Lines
...un/halo/app/security/device/DeviceServiceImpl.java 0.00% 23 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #6483      +/-   ##
============================================
+ Coverage     54.51%   58.22%   +3.71%     
- Complexity     3523     3778     +255     
============================================
  Files           646      651       +5     
  Lines         21862    22138     +276     
  Branches       1528     1555      +27     
============================================
+ Hits          11917    12890     +973     
+ Misses         9328     8625     -703     
- Partials        617      623       +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@ruibaby ruibaby left a comment

Choose a reason for hiding this comment

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

/lgtm

@f2c-ci-robot f2c-ci-robot bot added the lgtm Indicates that a PR is ready to be merged. label Aug 21, 2024
Copy link
Member

@JohnNiang JohnNiang left a comment

Choose a reason for hiding this comment

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

我还发现另外一个问题:在同一个浏览器中频繁切换账号会导致设备使用记录越来越多。

@guqing
Copy link
Member Author

guqing commented Aug 21, 2024

我还发现另外一个问题:在同一个浏览器中频繁切换账号会导致设备使用记录越来越多。

这是正常的,切换了账号之后设备id就不在了,目前检测设备是靠设备 id 的 cookie 的,设备记录超过10个会被清理

@JohnNiang
Copy link
Member

我还发现另外一个问题:在同一个浏览器中频繁切换账号会导致设备使用记录越来越多。

这是正常的,切换了账号之后设备id就不在了,目前检测设备是靠设备 id 的 cookie 的,设备记录超过10个会被清理

我仍然坚持我的观点:这种现象是不正常的。这种场景应该和登录后登出再登录的结果保持一致。不过这并不妨碍当前 PR 的合并。

Copy link
Member

@JohnNiang JohnNiang left a comment

Choose a reason for hiding this comment

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

/approve

Copy link

f2c-ci-robot bot commented Aug 22, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: JohnNiang

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@f2c-ci-robot f2c-ci-robot bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 22, 2024
@f2c-ci-robot f2c-ci-robot bot merged commit 87368df into halo-dev:main Aug 22, 2024
8 checks passed
@guqing guqing deleted the fix/device branch August 22, 2024 09:22
@ruibaby ruibaby modified the milestones: 2.19.x, 2.19.0 Aug 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/core Issues or PRs related to the Halo Core kind/bug Categorizes issue or PR as related to a bug. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants