Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
What the title says.
Comment | File | Size | Author |
---|
What the title says.
Comment | File | Size | Author |
---|
Comments
Comment #1
sunYou need the patch from #1555862: DrupalWebTestCase::drupalGetToken() does not add hash salt to make the test work.
Comment #2
wjaspers CreditAttribution: wjaspers commentedAre you proposing this goes into core or just that masquerade gets rolled ahead to D8's new module/plugin architecture?
Either way, there are a few lingering feature requests in the queue that would be worth rolling up.
Comment #3
andypostYay! Thanx a lot for #1555862
The only thing that scares me - masquerade as anonymous which could be dropped for core
For D8 there's a strange situation with session handling and we need more reviews for #1813696: Set/unset Session flag when masquerade status changes.
Next one is clean-up #1835954: Cleanup code and allow pass tests that broken because of token generation
Comment #4
sunCan you open up a 8.x-1.x branch and commit #0?
Yes, there are definitely quite a range of things that can be vastly simplified first, potentially dropped later.
Also, bug reports like #1813696: Set/unset Session flag when masquerade status changes. demand for test coverage. As soon as the core patch is in, the testbot should be enabled for the 8.x-1.x branch.
Comment #5
sunFWIW, I could also commit it myself? *wink* ;)
Comment #6
andypostI've commited your initial patch and created a new 8.x-1.x-dev release
Comment #7
deekayen CreditAttribution: deekayen commentedRe #3, if you've followed the masquerade history for a couple years, the masquerade as anonymous feature has been introduced, then pulled, then re-introduced, then pulled, in a cycle. It's never really been a good idea. My proposal is if you want to masquerade as anonymous, then logout. I'd prefer to make that policy stick and yank the functionality back out of masquerade.
Comment #8
sun+1 to that. I don't really see the point in masquerading as anonymous either.
Also, sorry for getting silent here. Work on this initial porting patch allowed me to study what exact functionality is currently supported by Masquerade module, and I've to admit, that's far more than I remotely expected.
Therefore, I was mainly busy with "thinking" ;) and trying to make sense of the questions:
Additionally, there's some functionality in Masquerade that I didn't expect to see, and for which I rather expected to see Devel module's rather simplistic behavior; namely, the (limited) list of default "switch users".
Comment #9
andypostThe "limited list of users" we have as block on per-user basis. Also there's a request to re-implement the logic to allow masquerade on specific roles only. #1171500: Add "masquerade as @role" permissions/settings for each role
I'd like to make integration for admin_menu the same as devel does because I always use both modules in my dayly job
Comment #10
sunYeah, to clarify some further from my side:
I'd ultimately like to replace admin_menu's built-in support for Devel (completely) with Masquerade. That is, because masquerading is what >80% of all developers/users actually want and need; switching user sessions is not exactly the desired functionality.
Meanwhile however, I rather strongly believe that the current Masquerade module is too complex and provides too many advanced features, options, and settings. Due to that, I would always think twice, before considering to install the module on a site.
I wonder whether you'd be open to do some larger changes? I'd imagine these steps:
What do you think?
Not having thought about it too much yet, but here are the components that I'd foresee in the basics:
A single user permission to "Masquerade as another user".
A (tl;dr) "widget" that contains a link "Masquerade". Supposed to be attached to the user account name in (whichever) toolbar.
This widget essentially allows for a few, discrete interactions:
In case the masqueraded user also has access to (whichever) toolbar, then the user account name receives some special styling to indicate so, and also allows to unmasquerade.
Otherwise (or any in case), the Account links (secondary links) menu is always enhanced to additionally contain a "Unmasquerade" link.
And that's... more or less all. No block. No user account settings. No role-specific advanced stuff, or whatnot.
Just the 80%, and just simply that.
Comment #11
andypostSure! The UI looks really awesome!
Agreed here, but probably better to move this "features" into plugins.
Suppose "masquerade as" fixed user list.
Or probably the "d8-plugins" enabled in settings form.
block & functions to [un]switch users.
Autocomplete also needs review because core has one & #1171500: Add "masquerade as @role" permissions/settings for each role suggests to filter results by allowed users only
Comment #12
sunOn plugins:
Well, hm. My baseline thinking is to significantly simplify the main masquerade.module. Yes, plugins are all-optional and stuff, but nevertheless, they directly imply more code that is loaded, more stuff that is exposed in the UI, more code to maintain, more complexity, and many more things.
Unfortunately, we did not meet the deadline for the D8 feature freeze here — but the essential point of this effort would be to
Everything that isn't part of the optimized 80%/main use-case is moved away into another (sub-?)module. The vast majority of users does not need that stuff and should not need it in the first place.
Everyone who installs Masquerade module should be able to say:
The last bullet might actually be the most important, as it summarizes what I tried to get to earlier:
If you look at the admin_menu + devel integration, then that's the exact experience I'm aiming for — There's only a single "switch users" permission, and that's it.
Thus, you just enable the modules, grant that permission, and waka-boom, you're fully set. There are no blocks to configure, no plugins in the block library that get in your way, no obscure fields or settings on user accounts, no giant list of complex permissions, nor anything else. It's 100% focused, immediately available when you need it, and very intuitive. The only problem with Devel is that Devel has no business on a production site. (Next to switching user sessions instead of masquerading.)
And that's where this revised Masquerade module comes in: Make it bad-ass simple. And rock solid. And nothing else.
Comment #13
sunAttached patch demonstrates what I'm after. (It's only the starting point though.)
53% of the code + complexity just simply goes away.
This impacts the most trivial and basic things only. There's a second ~25% chunk of code that can be vastly simplified.
Essentially, the goal is:
That said, honestly, I do not care at all for the 70-80% of add-on code and functionality that would be removed during this course. I only care for the most common + main 20% functionality, as outlined in #12. Therefore, someone else would have to pick up the responsibility for that removed code, and re-implement (and maintain and test) it in a separate sub-module or separate d.o project. (I'd prefer the latter.)
Or in other words: If you need more options, do it yourself. We'll make sure to provide a sophisticated API. The contributed masquerade_advanced.module might serve your needs, but I/we don't really care.
Comment #14
sunAlright. I discussed this some more with @andypost in IRC last night.
Attached patch does not make any further substantial progress towards #10 + #12, but instead of deleting all the existing code, the advanced functionality is moved into a new masquerade_advanced.module.
Note that I did not test the code (except of installing), because testing is obviously not possible without the actual new user interface of #10 + the deep integration into toolbar modules. Additionally, there's quite some code that needs to be updated for latest changes in core/HEAD. In other words, the main masquerade.module does not really do anything with this patch.
Nevertheless, I'd like to move forward with this patch and commit it as an initial milestone. Afterwards, I'd like to continue to work on the new UI/integration.
Speaking of, here's a more concrete plan I'd propose:
Create a new 8.x-2.x branch. Let's commit this patch to 8.x-2.x only.
This is a serious rewrite of Masquerade module, and a new major release will be appropriate on all fronts — it clarifies that there are API changes, that there are UI changes, and sends out the right message to everyone.
After completing work for 8.x-2.x, we can investigate to backport the 2.x series to D7.
I think I'd be interested in that. It would also help me to eliminate (or at least deprecate) the Devel integration in admin_menu a bit sooner.
Comment #15
andypost@sun Thanx a lot for your patient answers in IRC tonight!
I decided to move forward here with 8.x-2.x branch http://drupal.org/node/1926048
Also added @sun as maintainer!!!
The roadmap:
1) Split module on masquerade and masquerade_advanced
2) Masquerade module just simple api + toolbars integration
3) advanced module should add features like block, switch links, roles (#1171500: Add "masquerade as @role" permissions/settings for each role )
Questionable moments covered:
1) permissions: latest patch introduces hook_masquerade_access() which should solve access issues
2) api functions masquerade_switch_user() masquerade_switch_back() seems should not be involved in access checks
3) test coverage
Comment #16
sunThanks! :)
I've committed and pushed #14 to 8.x-2.x.
Attached patch serial:
I.e., with this patch, the basic functionality appears and is usable again. Note that I consider the block as a temporary measure only.
I also created #1926074: Remove {masquerade} table and rely on session flag only
Comment #17
sunAdditionally fixed the tests by working around the drupalGetToken() core bug for now.
Comment #18
andypostI think that's useless
This does not works for D7, so not sure D8 supports this
I think if we going to rename this so this name should be similar
switch|unswitch => masquerade|unmasquerade
Comment #19
andypostCommited conversion of HTTP_REFERER
Comment #20
sunSince #17 re-establishes the basic module functionality, I've committed that. :)
Also enabled the testbot for the 8.x-2.x branch. Yay!
Attached patch:
hook_user_operations()
implementation with a form alter to add a Masquerade operation link for each account on /admin/people.Comment #21
sunSorry, I just noticed that there's an additional flag for d.o projects that toggles whether the testbot is enabled.
Same as #20.
Comment #22
sunCommitted and pushed to 8.x-2.x. Yay! :)
Comment #23
andypost#21 is great idea. Tab on user page and removal of operations in favor of operations link (should be added to reeadme)
Also #1555862: DrupalWebTestCase::drupalGetToken() does not add hash salt still broken
Comment #24
sunRevamped masquerade access and UI validation.
Comment #25
sunCommitted #24.
Now: Added masquerade_user_is_masquerading() helper function.
Comment #26
sunCommitted #25.
Alrighty! With that, we have a pretty awesome clean slate :)
masquerade.module measures 13.8 KB now, which is ~46% of its original size. I'll look into #1926074: Remove {masquerade} table and rely on session flag only now, which will shave off another good chunk of bytes.
Comment #27
andypostA bit of cleanup needed
Comment #29
andypostSomehow global $user object could be not User entity
Comment #30
andypostCommited
Comment #31
sunA dozen of further clean-ups.
Some issues that occurred to me:
hook_user_view()
implementation + standalone link on the user account page sucks big time. I'd like to remove that in a follow-up patch after this.Comment #32
sunCommitted #31.
@andypost: I'd recommend to wait with a 8.x-2.x release (D8 will still change under the hood anyway...). Please note that I do not plan to work on the masquerade_advanced sub-module. Instead, I'll work on the following topics next:
That said, I'm done for today. ;) I first need to sleep over aforementioned topics either way. :)
Comment #33
deekayen CreditAttribution: deekayen commentedYou guys sure moved fast on this.
Reviewing whats been commited so far, I'm not clear whether the CSRF vulnerability has been re-introduced with the chopping. see http://drupal.org/node/835900
I see you've also maintained the masquerade as admin permission, which I want to kill. The permissions really need a full expansion of the grid so that each role can be allowed access to masquerade as one or more of the specific roles. If you're really going to continue with the "advanced" split, then the core module ought to have "masquerade as any user". The advanced part would allow the role breakout.
Comment #34
andypost@deekayen CRSF protection still there and also covered with tests now.
About "admin permission" (restricted) - I still think we can get rid of it. Now there's a hook_masquerade_access() which allows to implement "advanced grid" of rights which makes "restricted" is obviously useless
Comment #35
sun@deekayen:
Comment #36
sunFWIW, I pushed some initial changes for admin_menu 8.x-3.x to mute its existing Devel switch user integration in favor of Masquerade (if existent).
I've created a new feature branch 8.x-2.x-admin-menu, which already holds some basic (and working) integration code - mostly resembling the old switch user links (for now). Totally incomplete, and absolutely not fancy yet, but as said, at least it works (to some extent) already ;)
Not sure about the autocomplete form widget right within the menu yet, but that's exactly the smart and deep integration stuff we want to figure out and get right.
So if anyone feels eager to test this and wants to share UX feedback and ideas, please go ahead :)
(I just tried simplytest.me, but sadly, it did not pick up and provide the additional admin_menu module. Will try to notify @patrickd)
EDIT: Simplytest.me issue: #1927350: Additional module not downloaded in D8
Comment #37
moshe weitzman CreditAttribution: moshe weitzman commentedI haven't tried it yet, but it sounds like Masquerade 8 will be simple and strong. When masquerade is released, I'll remove devel's switch user feature.
Comment #38
sunThanks for chiming in, @moshe! Your offer to drop the Switch user feature from Devel is one more awesome incentive to make the new Masquerade kick ass :-)
EDIT: Background: #49398: Integrate in devel.module ? :)
I'm a bit swamped with $dayjob work currently, but I'm still tinkering about #32. So far, my thoughts mainly revolved around the question of "smart permissions" — here's a braindump thus far:
(roles - 1)²
permissions.'authenticated' + 'foo' + 'bar'
may masquerade as any other user that has all of these roles or a smaller subset of these roles. The user may not masquerade as another user who has these roles and the additional'baz'
role.'foo' + 'bar'
and User B with roles'bar' + 'baz'
, there's no clear vertical intersection between the two. We could allow this case, but only if the additional role of the target user does NOT contain any permissions that are tagged with'restrict access' => TRUE
inhook_permission()
. But I'm not 100% convinced of that.That said, I do believe that 3) + 4) cover the >80% case already. That is, because users, who are typically allowed to masquerade on a site, have higher level access permissions in the first place. In a most basic Drupal setup, that typically means users with the "administrator" role.
While I talked about roles in the above, it is possible that we want to base these security checks on the actual permissions of the associated roles instead. In particular, I'm thinking of the aforementioned "administrator" role, and that many sites are only granting this role to individual users - despite there being other roles - because this single role grants access to everything already. The same situation may arise when a site is not actually using the "administrator" role concept. In both cases, the list of roles is not meaningful. Only the difference/intersection of actual user permissions is the ultimate measure.
Comment #39
moshe weitzman CreditAttribution: moshe weitzman commentedI don't have real world experience with masquerade, but a quick ponder leads me to these simple rules:
1. Admin choose which roles are blacklisted. These are roles whose users are can't be switched into. Defaults to the uber Administator role
2. uid1 is always ineligible to be switched into.
3. Users with the single 'Use masquerade' permission can switch into any user account which is eligible (i.e. not blacklisted).
Comment #40
sunYeah,
2. is already the case; i.e., the default access mechanism does not allow anyone to masquerade as uid 1.
3. A single 'masquerade' user permission is implemented already. What doesn't exist is a blacklist condition. In fact, it existed before and we just removed that. ;)
And that sorta gets us to the point of "smart default permissions": As originally mentioned in #12, the higher level goal is to make Masquerade a no-brainer to set up:
A slightly funny and interesting aspect is that I'm essentially entering this from a Devel module perspective, which apparently is that simple to set up already: Tick a single permission, done.
In using Devel's Switch user functionality over the past ~5+ years (dunno when it got introduced), this always played an important factor in site + module setup processes: Don't make me think. Just enable it, tick that thing, ready to rumble.
Even more importantly, it did not matter whether I had an actual need for the functionality. And I think that's what made Devel's Switch user feature so popular: It is really trivial to set up, and thus it is there when you need (if you need it). The DX and UX consequences of that are huge.
So if technically possible in any way, I'd like to implant some smart and intelligent access logic, which allows us to 1) retain KISS with a single permission, 2) ensures (sane) security by default, and 3) doesn't make me think.
In particular, I'd like to get away without any configuration page or settings form, and also without a flood of
(n-1)^2
permissions. ;)Alas, there are two conditions (#38.3 + .4) that could be implemented very easily. :) The only point at which it gets complex is when the roles/permissions between source and target user do not intersect vertically, but horizontally instead.
That said, we could always decide to support those simple 3 + 4 cases only, and leave any more complex access rules to the masquerade_advanced.module (which can happily ship with a flood of permissions and also a dozen of configuration pages; doesn't matter — or rather, that's why it exists in the first place). That is, because again, our focus should be on serving and solving the >80% use-case.
Comment #41
sunSo... I actually think that this is the way to go.
More granular access permissions like #1171500: Add "masquerade as @role" permissions/settings for each role can be happily implemented by an add-on module, which may completely override the default access implementation (or may also choose to extend it instead). We can leverage Drupal's modular design exactly in the way it is supposed to be used: 1) Simple baseline + 2) Optional enhancement.
Also, FWIW, I've studied the scenarios and use-cases that have been mentioned over in that issue. Some of them are covered with this simple access mechanism already. Only those that need super fine-grained access control (role-per-role, per-user, blacklist) would need the add-on module.
Attached patch implements this simple access mechanism, as outlined before, in this exact order of conditions:
This patch comes with full test coverage for all permutations and test cases. Most noteworthy:
A) The typical 'administrator' role (with all permissions).
B) A 'moderator' role (with typical moderation + masquerade permissions).
C) An 'editor' role (with typical editor + masquerade permissions).
D) A 'masquerade' role (just that).
And it positively confirms this test plan:
In turn, we get away with a single user permission that is secure, simple, and smart, and we solve the >80% use-case.
Note that the patch is a little confusing due to renames=copies. I've pushed this code as 8.x-2.x-access feature branch.
Comment #42
sunSleeping over this, I'm still 100% confident that this is the most optimal way forward.
Thus, committed (with a small comment typo fix) to 8.x-2.x.
Comment #43
sunAlso committed docs and help text for the new built-in access control mechanism.
Comment #44
deekayen CreditAttribution: deekayen commentedI've been using Masquerade happily on production sites for years, but I think #41 turns it into a development-only module.
Take a site that has various companies or vendors who can login and place orders. They're assigned a Vendor role. I wouldn't want any of those vendors to login as a competing company's user and place fraudulent orders. Sure, they have the same access, but that doesn't mean that just because they want to masquerade as a user of the same role, that they should be allowed.
I understand you're trying to make this something that you can just turn on and it "works" without additional configuration, but without doing #1171500: Add "masquerade as @role" permissions/settings for each role alongside this change, you're breaking a lot of years of backwards functional expectation and compatibility across thousands of sites.
Comment #45
sunSure, I perfectly understand that there are advanced use-cases that require some more sophisticated access control mechanisms. But as mentioned before, the built-in default access control can be swapped out and/or enhanced easily.
A masquerade_advanced/masquerade_access [sub-?]module can provide these more granular access options. If done right, then this add-on module is equally small and simple, and solely focuses on solving exactly those advanced access requirements in the most clean and best way. For your particular use-cases, that's just going to be a click away to enable the add-on module.
Consequently, this is not a question of doing one thing OR another thing. — Instead, it's about sane defaults, vertical extensibility, and separation of concerns. In other words, we can easily have both, and no one is going to suffer.
The Masquerade module exposes a
hook_masquerade_access()
already, which follows thehook_node_access()
architecture. This hook allows modules to grant/deny masquerade access based on other/custom access rules. If desired, the built-in default hook implementation can even be eliminated entirely viahook_module_implements_alter()
.That said, I'm also happy to discuss the option of turning the access mechanism into a registered service instead of a hook. This would mean that there can only be one access implementation, and the default implementation can only be swapped out entirely with another, but not with multiple — which may or may not make sense.
In short, all of this is still possible, and we actually kept the need for those advanced use-cases in mind while revamping the code. The one and only difference is that these advanced options are no longer provided by the main module.
I'd really like to encourage you and all others to discuss which advanced real world use-cases should be covered by the add-on module, and how they should be ideally implemented from a UX and feature perspective. We have the following issues for that:
#1926876: Fix and rework the masquerade_advanced sub-module
#1171500: Add "masquerade as @role" permissions/settings for each role
The latter is a concrete implementation proposal already, but I actually think it would make a lot of sense to have a higher-level discussion about use-cases, concepts, and requirements first. That is, because the total combination of advanced access rules — role-per-role permissions, user-specific grants to other users/roles, global blacklist/whitelist, etc — involve plenty of options and conditions already, and the resulting cumulative set of access rules as well as their UX will be non-trivial to figure out for site maintainers. In turn, that also presents a security risk.
Speaking of security, if that advanced access implementation wants to live in the main Masquerade project, it will need 100% test coverage. If that's not going to be the case, then the module should live in a separate d.o project, since we'd not be able to make security guarantees otherwise.
Comment #46
deekayen CreditAttribution: deekayen commentedSpeaking in the context of your desire to get this added to core, I'm speaking to the darks side of your proposal, for the uninformed admins that will use the module.
I've made the case before, specifically about OpenID, that site admins will enable modules and have no idea what they do, let alone their related security implications. What we're talking about with your proposed default configuration for permissions is giving admins a module in core, which if enabled and not configured, turns the site into a development platform. It would grant to users in a forum the rights to impersonate another user during a flamewar, give vendors in a commerce site to place fake orders for a competitor, give employees the rights to delete their boss's account, give peers the rights to pretend to do code review in Open Atrium style sites, and so on.
Contributed modules will see Masquerade as a possible dependency. They'll require it to be enabled for some silly use of a single function that really has nothing to do with masquerading as another user, but then the site will have all the relevant impacts.
I think you're trying to make security easy, but security sucks. If on the other hand you were proposing to make #1171500: Add "masquerade as @role" permissions/settings for each role the default and have a addon module called masquerade_devel with your proposed permissions, that seems like it would make more sense to me.
Comment #47
tecto CreditAttribution: tecto commentedLatecomer to this conversation, but speaking from an information security perspective, the Masquerade module (in the current incarnation) falls into the category of a necessary evil. Masquerade violates a primary rule of information security: accountability. Users cannot be held accountable for their actions when those actions cannot be traced back to them. Masquerade exists because in spite of this violation, there are scenarios (testing during development, user support on live sites) that are significantly challenging without this option.
To compensate for this violation, it is appropriate to narrowly control the ability to masquerade as another user. Security is hard. The law of unintended consequences bites in this area with surprising frequency and cost. If we are going to knowingly violate a cardinal rule of information security, we should not mask the reality that we are very likely to introduce a security vulnerability.
An automatic grant of masquerade rights for any account that has matching or fewer permissions than the currently logged in user makes sense in a development (test/staging) environment. To distinguish the appropriate use, that functionality should be part of something like masquerade_devel.
Making the stock masquerade module function this way is a recipe for a site admin to enable the module for a specific need and not realize the fact that he has granted other users extraordinary access.
While a permissions page that looks like a wall of checkboxes is intimidating, it is appropriate. Just as the workflow module creates a wall of options as the workflow states and roles expand, permissions for masquerading appropriately grows more intimidating as the complexity of the many roles in a complex site grows. It conveys that the decisions to be made are non-trivial and should be considered carefully.
Comment #48
liquidcms CreditAttribution: liquidcms commentedalso late to this and sad to see Masquerade being neutered (as Admin Role was when feebly pulled in to core for D7). The way Masquerade works now (D7) is great. Just because 1 or 2 guys don't see need for a feature like masq as anon; doesn't mean it isn't useful.
I am all for simplifying code; but certainly not for reducing functionality.
Hoping we don't end up needing a Masquerade 2 to replace the lost functionality (like I had to do with AdminRole).
Comment #49
andypostIssue #2018831: There is no Type in masquerade_advanced.info.yml to enable advanced module
Comment #50
alexander_danilenko CreditAttribution: alexander_danilenko commentedHi guys!
When i place Masquerade block (/admin/structure/block/add/masquerade/bartik) i get an error
It seems we have to use
build()
method insteadblockBuild()
.It resolves error for me and block with user switcher form appears on page.
Review patch please.
Comment #51
andypostThanx, commited - this was caused by #2014215: Shift render array defaults back out onto BlockRenderController
Comment #52
alexander_danilenko CreditAttribution: alexander_danilenko commentedHello, guys! New patch due to new changes in Drupal 8 core (:
$user->uid
replaced with$user->id()
(only in masquerade.module)drupal_goto()
replaced withRedirectResponse
objectReview please ;)
Comment #53
andypostGreat step forward. but still needs work
$database needs proper injection. Your code passes container as a whole.
I'd recommend you to check core 8 for examples of this kind of injection
This is a callback so not needed at all. just remove
user_access() is deprecated. use $target_account->has_permission() instead.
suppose you just need to return $response
this change is not needed
please, add new line
Comment #54
andypostCommited a bunch of fixes
PS: tests still broken
Comment #55
andypostPushed 2 commits, module now installs but CSRF menu alters does not works.
Form moved to own file
Some code clean-ups
Comment #58
andypostRight now module mostly chasing HEAD
@todo is:
1) fix permissions
2) clean-up controlllers
3) extract switch to service helper class
4) add toolbar button and probably, modal form to switch
then
4) advanced module
I see this one as masquerade_ui or advanced that provides integration to third-party modules (action, condition, config form to blacklist roles, inject info user entity form/view, role form)
Comment #59
jhedstromThis patch is focused on getting the tests to a usable state. There are some failures with this, but they actually look like legitimate fails, rather than the current fails that are due to fatal errors.
Comment #60
jhedstromThe
user.admin_account
route was changed in latest Beta.Comment #61
andypost@jhedstrom are you able to run tests locally? module was broken in switching.
I can't test right now, but it seems makes sense to commit the patch to unfreeze testing, is not it?
Comment #62
jhedstrom@andypost running locally with the patch in #60 is able to complete the test run, whereas without the patch, fatal errors and exceptions prevent completion. There are a few fails, but they look to be like legitimate fails, rather than bugs in the tests. Attached is the output from the command:
php core/scripts/run-tests.sh --verbose --color --concurrency 4 --php `which php` --url http://d8.devl "masquerade" | tee /tmp/test.txt; TEST_EXIT=${PIPESTATUS[0]}; echo $TEST_EXIT
Comment #64
andypost@jhedstrom thanx, commited #60, that's much better then previous state, we need to unblock testing patches
Comment #66
andypostImplemented Masquerade service and cleaned-up procedural functions
@todo Remove
masquerade_switch_user_validate()
Comment #68
andypostSuppose to close the issue once access/validation logic from
masquerade_switch_user_validate()
will find it's home.Now a list of follow-ups
Comment #70
andypostFixed tests in
MasqueradeTest
- csrf seed is not accessible from parent site, test usingclickLink()
Also fixed autocomplete and redirect.
Somehow "Unmasquerade" link is always visible, needs work as well as
MasqueradeAccessTest
Comment #71
FrancewhoaThanks all for your contributions
We're happy to contribute testing patch, quality assurance, documentation, and agile project management services if needed
Related pages
Comment #77
andypostSuppose this issue fixed, so I filed beta 1 release https://www.drupal.org/node/2615552
Any other caches should go own issues.
To release RC version:
- #1926876: Fix and rework the masquerade_advanced sub-module
- #2448271: Improve module UI
- add more tests for #2448699: Implement caching for the masquerade block and links with a 'masquerade' cache context
- provide upgrade path after fixing #1171500: Add "masquerade as @role" permissions/settings for each role