This is a sub-task of http://drupal.org/node/1775842 Convert all variables to state and/or config systems.

Files: 
CommentFileSizeAuthor
#14 1826190-mac_key-cmi-14.patch3.02 KBalexpott
PASSED: [[SimpleTest]]: [MySQL] 48,194 pass(es).
[ View ]
#11 1826190-mac_key-cmi-11.patch2.12 KBvijaycs85
PASSED: [[SimpleTest]]: [MySQL] 48,194 pass(es).
[ View ]
#8 1826190-mac_key-cmi-7.patch2.64 KBLukas von Blarer
FAILED: [[SimpleTest]]: [MySQL] 47,465 pass(es), 88 fail(s), and 23 exception(s).
[ View ]
#6 1826190-mac_key-cmi-6.patch2.29 KBLukas von Blarer
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1826190-mac_key-cmi-6.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#3 1826190-mac_key-cmi-2.patch2.09 KBvijaycs85
FAILED: [[SimpleTest]]: [MySQL] 46,288 pass(es), 101 fail(s), and 25 exception(s).
[ View ]
#1 1826190-mac_key-cmi-1.patch1.51 KBvijaycs85
PASSED: [[SimpleTest]]: [MySQL] 46,360 pass(es).
[ View ]

Comments

vijaycs85’s picture

Status:Active» Needs review
StatusFileSize
new1.51 KB
PASSED: [[SimpleTest]]: [MySQL] 46,360 pass(es).
[ View ]
alexpott’s picture

Status:Needs review» Needs work

You need to set the mac_key in openid_test.install

+++ b/core/modules/openid/lib/Drupal/openid/Tests/OpenIDFunctionalTest.phpundefined
@@ -362,7 +362,7 @@ function testSignatureValidation() {
+    $association->mac_key = state()->get('mac_key') ?: FALSE;

There's no point doing the conditional here... should just be $association->mac_key = state()->get('mac_key');

vijaycs85’s picture

Status:Needs work» Needs review
StatusFileSize
new2.09 KB
FAILED: [[SimpleTest]]: [MySQL] 46,288 pass(es), 101 fail(s), and 25 exception(s).
[ View ]

Thanks for your comment. I don't see "openid_test_mac_key" that defined in openid_test.install used anywhere. So replacing it with "mac_key".

alexpott’s picture

Also the state key should be namespaced... so it should be openid_test.mac_key

alexpott’s picture

Status:Needs review» Needs work

It's interesting that the tests works without a mac_key... this should be investigated too.

Lukas von Blarer’s picture

Status:Needs work» Needs review
StatusFileSize
new2.29 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1826190-mac_key-cmi-6.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

I replaced the state key.

What should I do to investigate the testing issue?

Status:Needs review» Needs work

The last submitted patch, 1826190-mac_key-cmi-6.patch, failed testing.

Lukas von Blarer’s picture

Status:Needs work» Needs review
StatusFileSize
new2.64 KB
FAILED: [[SimpleTest]]: [MySQL] 47,465 pass(es), 88 fail(s), and 23 exception(s).
[ View ]

I had the wrong Git config...

Status:Needs review» Needs work

The last submitted patch, 1826190-mac_key-cmi-7.patch, failed testing.

Lukas von Blarer’s picture

The tests seem to be broken. Initially there were thwo variable names mac_key and openid_test_mac_key. And we are converting both of them to openid_test.mac_key. Is this correct? Also, We are not converting a set. This can't have the correct result in the tests.

vijaycs85’s picture

Status:Needs work» Needs review
StatusFileSize
new2.12 KB
PASSED: [[SimpleTest]]: [MySQL] 48,194 pass(es).
[ View ]

Keeping existing openid_test_mac_key as it is.

vijaycs85’s picture

Lukas you are right. Its not failing if I don't change the openid_test_mac_key I don't find any other instant of this variable anywhere other than install. Not sure why it is causing issue. But this is good to go with what we have at #11.

cam8001’s picture

Status:Needs review» Reviewed & tested by the community

Looks good to me!

alexpott’s picture

Status:Reviewed & tested by the community» Needs review
StatusFileSize
new3.02 KB
PASSED: [[SimpleTest]]: [MySQL] 48,194 pass(es).
[ View ]

Lets not perpetuate this bug...

+++ b/core/modules/openid/tests/openid_test.installundefined
@@ -13,5 +13,5 @@ function openid_test_install() {
// Generate a MAC key (Message Authentication Code) used for signing messages.
// The variable is base64-encoded, because variables cannot contain non-UTF-8
// data.
- variable_set('openid_test_mac_key', base64_encode(_openid_get_bytes(20)));
+ state()->set('openid_test_mac_key', base64_encode(_openid_get_bytes(20)));

The .install file and variable/state key is completely unnecessary as it is in never actually used!

How about this patch...

vijaycs85’s picture

+1 to Alex patch... we don't use it anyway.

cam8001’s picture

Status:Needs review» Reviewed & tested by the community

Taking code out is always better than putting code in :)

catch’s picture

Title:Covert mac_key variable to CMI system» Remove dead mac_key variable from open_id_test module
Status:Reviewed & tested by the community» Fixed

Heh nice find, let's kill it. Committed/pushed to 8.x.

Automatically closed -- issue fixed for 2 weeks with no activity.

vijaycs85’s picture

Assigned:vijaycs85» Unassigned
Issue summary:View changes