Allow to identify whether a user is logged without presence of Account menu links as secondary links [#1894692]

This blocks: #1869476: Convert global menus (primary links, secondary links) into blocks

Problem

There is no way to identify whether a user is logged in, without the presence of Account menu links as secondary links on a HTML page.

Goal

Allow Web Tests to uniquely identify whether a user is logged in.

Details

Tests need to be able to identify whether a user that just logged in, is actually logged in.
Tests are currently doing that via: $this->assertText(t('Log out')); — the unique "Log out" link that appears in the account menu when a user is logged in.
#1869476: Convert global menus (primary links, secondary links) into blocks turns the primary links + secondary links into block plugins, which means that there are no secondary links by default anymore. (Which is awesome. Less assumptions++)

Proposed solution A

Add a HTML meta tag to the HTML page markup.
(Is there a meta like this? I briefly looked around, but didn't find something that would match.)

Proposed solution B

Develop a fancy test helper method, which reverse-engineers the Session ID from the HTTP Response headers to double check whether there is a corresponding session in the database, whether it's valid + active + not expired, and whether it belongs to the user account in question.

Comment	File	Size	Author
#2	test.login_.2.patch	4.32 KB	sun
#2
#2	interdiff.txt	851 bytes	sun
#1	test.login_.1.patch	4.32 KB	sun
#1

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

sun

German

Karlsruhe

CreditAttribution: sun commented 21 January 2013 at 18:43

Assigned:	Unassigned	» sun
Status:	Active	» Needs review

File	Size
test.login_.1.patch	4.32 KB

Attached patch implements proposal B.

Comment #2

sun

German

Karlsruhe

CreditAttribution: sun commented 21 January 2013 at 18:48

File	Size
interdiff.txt	851 bytes
test.login_.2.patch	4.32 KB

Provide a negative assertion in case user login fails.

Comment #3

larowlan

🇦🇺🏝.au GMT+10

CreditAttribution: larowlan commented 21 January 2013 at 19:41

I've seen some themes that add a class to the body element, logged-in
Could we add the same in template preprocess html?

Comment #4

sun

German

Karlsruhe

CreditAttribution: sun commented 21 January 2013 at 19:49

Hm! You're genius, and you're right - that class exists in core, even :-D

@see first lines of template_preprocess_html()

Should we rely on that instead of HTTP response header stuff?

Pro: Trivial.

Con: Not reliable. Just moving the problem elsewhere. A test theme or template variable preprocessor overriding the logged_in template variable will break it.

What do you think?

Comment #5

sun

German

Karlsruhe

CreditAttribution: sun commented 21 January 2013 at 19:59

Oh wow.

I actually did not expect #1 to come back green. ;)

But given that it did, #2 will likely, too, so I think I'd prefer the reliable HTTP/session-based solution. :)

Comment #6

larowlan

🇦🇺🏝.au GMT+10

CreditAttribution: larowlan commented 21 January 2013 at 20:07

I'm with you on choosing the least fragile route

Comment #7

sun

German

Karlsruhe

CreditAttribution: sun commented 21 January 2013 at 21:03

Was that an RTBC? :)

Comment #8

larowlan

🇦🇺🏝.au GMT+10

CreditAttribution: larowlan commented 21 January 2013 at 21:12

Will review properly shortly, will also get feedback from others on irc.

Comment #9

xjm

she/her

English

CreditAttribution: xjm commented 21 January 2013 at 21:15

Status:

Needs review

» Reviewed & tested by the community

Looks great to me!

Comment #10

larowlan

🇦🇺🏝.au GMT+10

CreditAttribution: larowlan commented 21 January 2013 at 21:23

I'm happy with #2, it too turned out to be fairly trivial in the end.

Comment #11

Dries CreditAttribution: Dries commented 23 January 2013 at 17:19

Status:

Reviewed & tested by the community

» Fixed

I really think we should go with approach B, and have good APIs for things like checking whether a user is logged in. Glad we're going down the path of option B.

+++ b/core/modules/simpletest/lib/Drupal/simpletest/WebTestBase.phpundefined
@@ -632,16 +632,31 @@ protected function drupalLogin($user) {
+    // @see WebTestBase::drupalUserIsLoggedIn()
+    if (isset($this->session_id)) {
+      $user->session_id = $this->session_id;

This freaked me out for a bit but it made sense in the end. I think renaming $user to $account would make me much more comfortable.

Committed to 8.x.