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

PROD-31305: Non verified users are not able to see their profile properly #4236

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

vcsvinicius
Copy link
Contributor

@vcsvinicius vcsvinicius commented Dec 6, 2024

Problem (for internal)

Non verified user can't access they own profile information page.

Solution (for internal)

I created a custom Access View for check the access for User Information page.

Release notes (to customers)

Fixed the access for non-verified user on Profile User Information page.

Issue tracker

PROD-31305

Theme issue tracker

N/A

How to test

  • Create a non-verified user
  • Log in with non-verified user
  • Access page: /user/{user-id}/information

Change Record

N/A

Translations

N/A

@vcsvinicius vcsvinicius added type: bug Fixes a bug in Open Social status: needs review This pull request is waiting for a requested review prio: medium team: guardians labels Dec 6, 2024
@vcsvinicius vcsvinicius added this to the 13.0.0-alpha20 milestone Dec 6, 2024
@vcsvinicius vcsvinicius requested a review from a team December 6, 2024 15:15
@vcsvinicius vcsvinicius force-pushed the bugfix/PROD-31305/fix-access-to-own-profile-non-verified-user branch 2 times, most recently from 2edea62 to badf9d0 Compare December 9, 2024 19:10
Copy link
Member

@Kingdutch Kingdutch left a comment

Choose a reason for hiding this comment

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

Nice find on where the issue was coming from with the custom code in social_user.

There is a potential security issue in here and I think we should reconsider why we need _custom_access here.

Comment on lines 71 to 90
// Load the user entity.
$user_id = $this->routeMatch->getRawParameter('user');
$user = $this->entityTypeManager->getStorage('user')
->load($user_id);
if (!$user instanceof UserInterface) {
return AccessResult::forbidden();
}

// If user is blocked, check special permission.
if ($user->isBlocked()) {
return AccessResult::allowedIfHasPermissions($account, ['view blocked user']);
}

// Check if current page is from logged user.
if ($account->id() === $user->id()) {
return AccessResult::allowedIfHasPermissions($account, ['view own profile profile']);
}

// Check if logged user can view all profiles.
return AccessResult::allowedIfHasPermissions($account, ['view any profile profile']);
Copy link
Member

Choose a reason for hiding this comment

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

How does this logic differ from what would be evaluated if we called $profile->access('view', $account);?

What happens if other modules change when a user can be viewed by using Drupal's access APIs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I debugged with entity-access instead of custom-access and I found where and why the isn't give access.

The validation happen here: html/core/lib/Drupal/Core/Entity/EntityAccessCheck.php:65
The code try to found a parameter with profile, but the view has user as parameter, so to work we need to add this on view.

image

@vcsvinicius vcsvinicius force-pushed the bugfix/PROD-31305/fix-access-to-own-profile-non-verified-user branch from badf9d0 to 6c5b246 Compare December 10, 2024 14:00
@vcsvinicius vcsvinicius force-pushed the bugfix/PROD-31305/fix-access-to-own-profile-non-verified-user branch from 6c5b246 to c578657 Compare December 10, 2024 17:45
@vcsvinicius vcsvinicius force-pushed the bugfix/PROD-31305/fix-access-to-own-profile-non-verified-user branch from c578657 to 90ef4d0 Compare December 10, 2024 18:25
@BiaInacio
Copy link
Contributor

Non-verified user is able to access their information:
image

@open-social-tugboat
Copy link

Tugboat has finished building the preview for this pull request!

Link:

Dashboard:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
prio: medium status: needs review This pull request is waiting for a requested review team: guardians type: bug Fixes a bug in Open Social
Development

Successfully merging this pull request may close these issues.

5 participants