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

PoC: Return contact ID when logged in via checksum #21642

Closed

Conversation

mattwire
Copy link
Contributor

Overview

CRM_Core_Session::getLoggedInContactID() returns NULL when a contact is "logged in" via a checksum link. This doesn't really make sense because they can do everything that they would have been able to do if "logged in" via a normal session.

I've seen a lot of code that makes the assumption that

if (CRM_Core_Session::getLoggedInContactID() === NULL) {
  // do anonymous user stuff
}

Inspired by the new loginsecurity extension by @mlutfy

Before

getLoggedInContactID() doesn't always do what you expect (if logged in via checksum).

After

Default behaviour is not changed. It is explicit because the parameter makes it clear that you will not get the checksum Contact ID unless you specifically request that behaviour.

Technical Details

Comments

@civibot civibot bot added the master label Sep 27, 2021
@civibot
Copy link

civibot bot commented Sep 27, 2021

(Standard links)

@mattwire mattwire marked this pull request as draft September 27, 2021 12:15
@mlutfy
Copy link
Member

mlutfy commented Sep 29, 2021

Thanks for raising this. I have to admit I've run into checksum inconsistencies a few times.

Are there legitimate use-cases for ignoring the checksum? i.e. for requiring a function parameter? Otherwise as a first step, we could create a separate function? (so that extensions such as loginsecurity do not improvise/duplicate their own validation code).

Test fails are weird:

    CRM_Contribute_Form_ContributionTest.testSubmit with data set #0
    CRM_Contribute_Form_ContributionTest.testSubmit with data set #1
    CRM_Contribute_Form_ContributionTest.testSubmitEmailReceiptUserEmailFromAddress
    CRM_Contribute_Form_ContributionTest.testEmailReceiptOnPayLater
    CRM_Contribute_Form_ContributionTest.testPartialPaymentWithCreditCard
    CRM_Contribute_Form_ContributionTest.testCardTypeAndPanTruncation

@mlutfy
Copy link
Member

mlutfy commented Sep 29, 2021

jenkins, test this please

@demeritcowboy
Copy link
Contributor

This has been WIP for a few months so going to close.

I'm personally not clear on what "logged in via checksum" means with respect to permissions when there is no associated cms user to look them up for, so that may be why there's some inconsistencies.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants