Re: [Apache Bloodhound] #667: Error rendering forbidden pages
#667: Error rendering forbidden pages +- Reporter: olemis | Owner: olemis Type: defect | Status: closed Priority: blocker| Milestone: Release 8 Component: ui design |Version: 0.7.0 Resolution: fixed | Keywords: permissions, chrome +- Changes (by rjollos): * status: assigned = closed * resolution: = fixed Comment: Part of the test case is fine because a product is created that isn't used in any other tests and therefore we don't have to worry about coupling. However we shouldn't change the test environment just because you can find some cases where this is done in the Trac codebase. Changes to //trac.ini// and the permissions system should generally be restored at the end of the test case. The test case doesn't fail when run against the codebase prior to r1528323 because there are still navigation items rendered due to anonymous having permissions at global scope. The test case was fixed and confirmed to failed against r1528322 before it was committed. --- (In [1542473]) 0.8dev: Added a functional test for rendering forbidden pages. Refs #667. -- Ticket URL: https://issues.apache.org/bloodhound/ticket/667#comment:17 Apache Bloodhound https://issues.apache.org/bloodhound/ The Apache Bloodhound issue tracker
Re: [Apache Bloodhound] #667: Error rendering forbidden pages
#667: Error rendering forbidden pages
+-
Reporter: olemis | Owner: olemis
Type: defect | Status: assigned
Priority: blocker| Milestone: Release 8
Component: ui design |Version: 0.7.0
Resolution: | Keywords: permissions, chrome
+-
Comment (by olemis):
Replying to [comment:12 rjollos]:
I applied [attachment:t667_r1526822_error_mainnav.tests.diff​], executed
the functional tests and saw the following error:
{{{
==
FAIL: runTest (tests.functional.admin.RegressionTestBhTicket667)
No error after removing all permissions
--
Traceback (most recent call last):
File
/home/user/Workspace/bhdev/bloodhound.git/bloodhound_multiproduct/tests/functional/admin.py,
line 42, in runTest
prefix = tester.create_product()
File
/home/user/Workspace/bhdev/bloodhound.git/bloodhound_multiproduct/tests/functional/__init__.py,
line 740, in create_product
tc.formvalue('new', 'action', 'new')
File
/home/user/Workspace/bhdev/bloodhound.git/trac/trac/tests/functional/better_twill.py,
line 153, in better_formvalue
raise twill.errors.TwillAssertionError(*args)
TwillAssertionError: ('no matching forms!',
'/home/user/Workspace/bhdev/bloodhound.git/trac/testenv/trac/log/RegressionTestBhTicket667.html')
--
}}}
That's what attachment:t598_r1526159_form_id.diff:ticket:598 is for .
I'm not sure what led to the error,
Have you applied that patch ? See comment:8
but the bigger problem I see is that revoking all permissions for
anonymous and not restoring them after the test is finished would affect
other functional tests.
That's normal with functional test cases . They are not as isolated as
unit tests .
It reminds me though, I have been considering a change to the Trac
functional test environment whereby all permissions would be revoked from
`anonymous` and `authenticated`.
In principle I see this as useful ...
I think that change would make it easier to write tests by allowing a
user to be created and their permissions to be explicitly set - they
wouldn't inherit any permissions.
... mainly because of this ...
It would probably require many changes to existing Trac functional
tests, however.
... but this is also true .
--
Ticket URL: https://issues.apache.org/bloodhound/ticket/667#comment:13
Apache Bloodhound https://issues.apache.org/bloodhound/
The Apache Bloodhound issue tracker
Re: [Apache Bloodhound] #667: Error rendering forbidden pages
#667: Error rendering forbidden pages
+-
Reporter: olemis | Owner: olemis
Type: defect | Status: assigned
Priority: blocker| Milestone: Release 8
Component: ui design |Version: 0.7.0
Resolution: | Keywords: permissions, chrome
+-
Comment (by olemis):
Replying to [comment:14 rjollos]:
Replying to [comment:13 olemis]:
Have you applied that patch ? See comment:8
The issue was fixed in [1542165 ].
Hmmm ... I'll take a look into this then .
;)
but the bigger problem I see is that revoking all permissions for
anonymous and not restoring them after the test is finished would affect
other functional tests.
That's normal with functional test cases . They are not as isolated as
unit tests .
Because they are not isolated you need to restore the environment state
at the end of each test case. Otherwise, you are potentially causing
problems for the next test case. We make the effort in the Trac test suite
to restore the environment state at the end of each test case. The same
should be done in Bloodhound. For example, see
[http://trac.edgewall.org/browser/branches/1.0-stable/trac/ticket/tests/functional.py?marks=559,573rev=12227#L553
TestAdminComponentNoneDefined].
OTOH by reviewing
[https://issues.apache.org/bloodhound/browser/trunk/trac/trac/ticket/tests/functional.py#L1159
RegressionTestTicket4630a] and
[https://issues.apache.org/bloodhound/browser/trunk/trac/trac/ticket/tests/functional.py#L1187
RegressionTestTicket4630b] it seems that the side effects of the former
are a pre-condition for executing the later . That's quite common in
functional test cases .
{{{#!sh
$ grep -n -r class .*b( ../trac/trac/ticket/tests/
../trac/trac/ticket/tests/functional.py:1187:class
RegressionTestTicket4630b(FunctionalTestCaseSetup):
../trac/trac/ticket/tests/functional.py:1250:class
RegressionTestTicket5394b(FunctionalTwillTestCaseSetup):
../trac/trac/ticket/tests/functional.py:1294:class
RegressionTestTicket5497b(FunctionalTwillTestCaseSetup):
../trac/trac/ticket/tests/functional.py:1491:class
RegressionTestTicket6879b(FunctionalTwillTestCaseSetup):
../trac/trac/ticket/tests/functional.py:1518:class
RegressionTestTicket6912b(FunctionalTwillTestCaseSetup):
}}}
In general the state most usually restored is the admin permissions for
admin user and keeping the tester logged in as admin .
--
Ticket URL: https://issues.apache.org/bloodhound/ticket/667#comment:15
Apache Bloodhound https://issues.apache.org/bloodhound/
The Apache Bloodhound issue tracker
Re: [Apache Bloodhound] #667: Error rendering forbidden pages
#667: Error rendering forbidden pages
+-
Reporter: olemis | Owner: olemis
Type: defect | Status: assigned
Priority: blocker| Milestone: Release 8
Component: ui design |Version: 0.7.0
Resolution: | Keywords: permissions, chrome
+-
Changes (by rjollos):
* status: review = assigned
* owner: rjollos = olemis
Comment:
I applied [attachment:t667_r1526822_error_mainnav.tests.diff​], executed
the functional tests and saw the following error:
{{{
==
FAIL: runTest (tests.functional.admin.RegressionTestBhTicket667)
No error after removing all permissions
--
Traceback (most recent call last):
File
/home/user/Workspace/bhdev/bloodhound.git/bloodhound_multiproduct/tests/functional/admin.py,
line 42, in runTest
prefix = tester.create_product()
File
/home/user/Workspace/bhdev/bloodhound.git/bloodhound_multiproduct/tests/functional/__init__.py,
line 740, in create_product
tc.formvalue('new', 'action', 'new')
File
/home/user/Workspace/bhdev/bloodhound.git/trac/trac/tests/functional/better_twill.py,
line 153, in better_formvalue
raise twill.errors.TwillAssertionError(*args)
TwillAssertionError: ('no matching forms!',
'/home/user/Workspace/bhdev/bloodhound.git/trac/testenv/trac/log/RegressionTestBhTicket667.html')
--
}}}
I'm not sure what led to the error, but the bigger problem I see is that
revoking all permissions for anonymous and not restoring them after the
test is finished would affect other functional tests.
It reminds me though, I have been considering a change to the Trac
functional test environment whereby all permissions would be revoked from
`anonymous` and `authenticated`. I think that change would make it easier
to write tests by allowing a user to be created and their permissions to
be explicitly set - they wouldn't inherit any permissions. It would
probably require many changes to existing Trac functional tests, however.
--
Ticket URL: https://issues.apache.org/bloodhound/ticket/667#comment:12
Apache Bloodhound https://issues.apache.org/bloodhound/
The Apache Bloodhound issue tracker
Re: [Apache Bloodhound] #667: Error rendering forbidden pages
#667: Error rendering forbidden pages +- Reporter: olemis | Owner: rjollos Type: defect | Status: review Priority: blocker| Milestone: Release 8 Component: ui design |Version: 0.7.0 Resolution: | Keywords: permissions, chrome +- Comment (by rjollos): (In [1528323]) 0.8dev: Prevent internal error rendering an error page and `mainnav` is not in `req.chrome['nav']`. Refs #667. Patch by Olemis Lang. -- Ticket URL: https://issues.apache.org/bloodhound/ticket/667#comment:11 Apache Bloodhound https://issues.apache.org/bloodhound/ The Apache Bloodhound issue tracker
Re: [Apache Bloodhound] #667: Error rendering forbidden pages
#667: Error rendering forbidden pages +- Reporter: olemis | Owner: olemis Type: defect | Status: accepted Priority: blocker| Milestone: Release 8 Component: ui design |Version: 0.7.0 Resolution: | Keywords: permissions, chrome +- Changes (by olemis): * status: assigned = accepted * owner: rjollos = olemis -- Ticket URL: https://issues.apache.org/bloodhound/ticket/667#comment:7 Apache Bloodhound https://issues.apache.org/bloodhound/ The Apache Bloodhound issue tracker
Re: [Apache Bloodhound] #667: Error rendering forbidden pages
#667: Error rendering forbidden pages
+-
Reporter: olemis | Owner: rjollos
Type: defect | Status: review
Priority: blocker| Milestone: Release 8
Component: ui design |Version: 0.7.0
Resolution: | Keywords: permissions, chrome
+-
Comment (by rjollos):
The patch appears to be effectively the same as the one in comment:3. Is
there a difference I'm not seeing, or just style differences? I'm fine
with using the `.get('mainnav', []))` since I think it's cleaner.
Is the `setdefault` needed? There won't be any iteration if `mainnav`
isn't in the dictionary, so it doesn't appear that the line with
`setdefault` would even be hit.
--
Ticket URL: https://issues.apache.org/bloodhound/ticket/667#comment:9
Apache Bloodhound https://issues.apache.org/bloodhound/
The Apache Bloodhound issue tracker
Re: [Apache Bloodhound] #667: Error rendering forbidden pages
#667: Error rendering forbidden pages +- Reporter: olemis | Owner: rjollos Type: defect | Status: assigned Priority: blocker| Milestone: Release 8 Component: ui design |Version: 0.7.0 Resolution: | Keywords: permissions, chrome +- Changes (by olemis): * owner: olemis = rjollos * status: needinfo = assigned -- Ticket URL: https://issues.apache.org/bloodhound/ticket/667#comment:6 Apache Bloodhound https://issues.apache.org/bloodhound/ The Apache Bloodhound issue tracker
Re: [Apache Bloodhound] #667: Error rendering forbidden pages
#667: Error rendering forbidden pages +- Reporter: olemis | Owner: olemis Type: defect | Status: needinfo Priority: blocker| Milestone: Release 8 Component: ui design |Version: 0.7.0 Resolution: | Keywords: permissions, chrome +- Comment (by olemis): Replying to [comment:3 rjollos]: Replying to [ticket:667 olemis]: The error message was displayed after revoking permissions to view target resource. I've tried revoking `TICKET_VIEW` and navigating to `/query`, but haven't been able to reproduce. Could you give some more specifics? What is the target resource? I recall that this happened after revoking PRODUCT_VIEW for anonymous , then logout and access the home page . Anyway, I have in mind to apply the following patch, but I'd like to reproduce the issue just to be sure: + -- Ticket URL: https://issues.apache.org/bloodhound/ticket/667#comment:5 Apache Bloodhound https://issues.apache.org/bloodhound/ The Apache Bloodhound issue tracker
Re: [Apache Bloodhound] #667: Error rendering forbidden pages
#667: Error rendering forbidden pages +- Reporter: olemis | Owner: olemis Type: defect | Status: needinfo Priority: blocker| Milestone: Release 8 Component: ui design |Version: 0.7.0 Resolution: | Keywords: permissions, chrome +- Changes (by rjollos): * status: accepted = needinfo * owner: rjollos = olemis -- Ticket URL: https://issues.apache.org/bloodhound/ticket/667#comment:4 Apache Bloodhound https://issues.apache.org/bloodhound/ The Apache Bloodhound issue tracker
Re: [Apache Bloodhound] #667: Error rendering forbidden pages
#667: Error rendering forbidden pages
+-
Reporter: olemis | Owner: rjollos
Type: defect | Status: accepted
Priority: blocker| Milestone: Release 8
Component: ui design |Version: 0.7.0
Resolution: | Keywords: permissions, chrome
+-
Comment (by rjollos):
Replying to [ticket:667 olemis]:
The error message was displayed after revoking permissions to view
target resource.
I've tried revoking `TICKET_VIEW` and navigating to `/query`, but haven't
been able to reproduce. Could you give some more specifics? What is the
target resource?
Anyway, I have in mind to apply the following patch, but I'd like to
reproduce the issue just to be sure:
{{{#!diff
===
--- bloodhound_theme/bhtheme/theme.py (revision 1525323)
+++ bloodhound_theme/bhtheme/theme.py (working copy)
@@ -253,10 +253,11 @@
req.href.wiki = hwiki
# Move 'admin' entry from mainnav to metanav
-for i, entry in enumerate(req.chrome['nav']['mainnav']):
-if entry['name'] == 'admin':
-req.chrome['nav']['metanav'] \
-.append(req.chrome['nav']['mainnav'].pop(i))
+if 'mainnav' in req.chrome['nav']:
+for i, entry in enumerate(req.chrome['nav']['mainnav']):
+if entry['name'] == 'admin':
+req.chrome['nav']['metanav'] \
+.append(req.chrome['nav']['mainnav'].pop(i))
return handler
}}}
--
Ticket URL: https://issues.apache.org/bloodhound/ticket/667#comment:3
Apache Bloodhound https://issues.apache.org/bloodhound/
The Apache Bloodhound issue tracker
Re: [Apache Bloodhound] #667: Error rendering forbidden pages
#667: Error rendering forbidden pages +- Reporter: olemis | Owner: nobody Type: defect | Status: new Priority: blocker| Milestone: Release 8 Component: ui design |Version: 0.7.0 Resolution: | Keywords: permissions, chrome +- Changes (by olemis): * component: = ui design * priority: major = blocker * owner: = nobody * version: = 0.7.0 * milestone: = Release 8 * keywords: = permissions, chrome -- Ticket URL: https://issues.apache.org/bloodhound/ticket/667#comment:1 Apache Bloodhound https://issues.apache.org/bloodhound/ The Apache Bloodhound issue tracker
Re: [Apache Bloodhound] #667: Error rendering forbidden pages
#667: Error rendering forbidden pages +- Reporter: olemis | Owner: rjollos Type: defect | Status: accepted Priority: blocker| Milestone: Release 8 Component: ui design |Version: 0.7.0 Resolution: | Keywords: permissions, chrome +- Changes (by rjollos): * owner: nobody = rjollos * status: new = accepted Comment: I suppose it is a regression from r1522789. I will take a look. -- Ticket URL: https://issues.apache.org/bloodhound/ticket/667#comment:2 Apache Bloodhound https://issues.apache.org/bloodhound/ The Apache Bloodhound issue tracker
