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

Username changes should be visible immediately (allUsers should prefer certificates over CSRs) #1895

Closed
Tracked by #1902
leblowl opened this issue Oct 3, 2023 · 7 comments
Labels
bug Something isn't working security

Comments

@leblowl
Copy link
Contributor

leblowl commented Oct 3, 2023

displayableCurrentChannelMessages selector uses allUsers to select the username for a message. If CSRs have precedence over certs, then it could display an invalid/old username for that user.

allUsers is also used in displayMessageNotificationSaga and showNotificationSaga to get the username for a notification.

@holmesworcester holmesworcester moved this to Sprint in Quiet Oct 3, 2023
@holmesworcester holmesworcester added bug Something isn't working security labels Oct 3, 2023
@holmesworcester
Copy link
Contributor

This relates to which username will show when a user has a pending request to change their username: their new requested name or their existing name.

@leblowl
Copy link
Contributor Author

leblowl commented Nov 13, 2023

@holmesworcester Do you have an idea for how to move forward with this?

Should we show the registered username until a pending username is registered? Or should we show a pending username if there is one? I think currently, we are doing the latter.

Also do you think we can address this later? To me, it doesn't seem like it should hold up the release necessarily, but curious to hear your thoughts.

@holmesworcester
Copy link
Contributor

I think showing the pending username is correct. If that's the current behavior and there aren't other questions let's close this.

@leblowl
Copy link
Contributor Author

leblowl commented Nov 13, 2023

I'm not sure if it is the current behavior. Looking at the code again, it looks like it prefers the cert:

Object.keys(certs).map(pubKey => {
users[pubKey] = {
...certs[pubKey],
isRegistered: true,
isDuplicated: false,
pubKey,
}
console.log('Unregistered Debug - allUsers selector - certs - user', users[pubKey])
})
Object.keys(csrs).map(pubKey => {
if (users[pubKey]) return

@holmesworcester
Copy link
Contributor

holmesworcester commented Nov 13, 2023

Then let's leave it open but move it out of this sprint.

I renamed the issue to make it clear what the product-level impact is.

@holmesworcester holmesworcester moved this from Sprint to Backlog - Desktop & Backend in Quiet Nov 13, 2023
@holmesworcester holmesworcester changed the title allUsers should prefer certificates over CSRs Username changes should be visible immediately (allUsers should prefer certificates over CSRs) Nov 13, 2023
@leblowl
Copy link
Contributor Author

leblowl commented Nov 13, 2023

Sounds good, thanks!

@holmesworcester holmesworcester moved this from Backlog - Desktop & Backend to Backlog - Mobile in Quiet Dec 5, 2023
@holmesworcester holmesworcester moved this from Backlog - Mobile to Backlog - Desktop & Backend in Quiet Dec 5, 2023
@holmesworcester
Copy link
Contributor

Deprecating this approach to usernames. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working security
Projects
None yet
Development

No branches or pull requests

2 participants