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

Prevent misuse of show_noauth param when fulltext is empty #11290

Merged
merged 4 commits into from
Feb 8, 2017

Conversation

ggppdk
Copy link
Contributor

@ggppdk ggppdk commented Jul 25, 2016

Pull Request for Issue #11285

Summary of Changes

Natural place to fix this is at the view.html.php that has a similar but incomplete check

Testing Instructions

STEP 1: Verify bug

  1. Enable "Show unauthorized links" (e.g. globally for articles component)
  2. Set an article to non-public access e.g. 'Registered', and make sure that the article does not have read-more
  3. Visit the article view as GUEST and confirm that the all article text is showing

STEP 2: Test fix: Redirect guests to login
4. Apply the patch and visit the article view again, you should redirected to login
5. Login as a "registered" user
6. Visit article, you should be able to view the article

STEP 3: Test fix: If logged user still did not gain access after login then a no access message work for logged users too
7. Edit article and set access to special
8. Visit article as "plain" registered users, you should get a 403 error

@brianteeman
Copy link
Contributor

Can you take a look at the codestyle issues please

@AlexRed
Copy link
Contributor

AlexRed commented Jul 31, 2016

ok for single article URL, but not for category blog view

@ggppdk
Copy link
Contributor Author

ggppdk commented Jul 31, 2016

I have corrected code styling issues,

About :

ok for single article URL, but not for category blog view

that can be a different PR ?

@hardiktailored
Copy link

(1) Issue verified.
(2) After applying patch, visiting article redirects to login page with redirect back link to article.
(3) Shows 403 error when accessing article by "registered" user when article have only "special" access allowed. This is when I visit single article menu item but visiting article by clicking from "Latest Articles" it only shows "Log Out" button instead of 403 error. See screen-shot:
screen shot 2016-08-04 at 06 07 58

(4) Article still visible from category blog layout. Another case is when we visit any public article without login and we click on category name from this article breadcrumb, all articles gets visible though one allowed to registered user only.


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11290.

@ggppdk
Copy link
Contributor Author

ggppdk commented Aug 4, 2016

(3) Shows 403 error when accessing article by "registered" user when article have only "special" access allowed. This is when I visit single article menu item

The 403 is the desired thing since there is no fulltext, so we are good here

(3) ... but visiting article by clicking from "Latest Articles" it only shows "Log Out" button instead of 403 error.

going to login screen should only happen if user is guest (i need to check this !), i am using:

if ($this->user->get('guest'))

(4) Article still visible from category blog layout

of course it is shown

  • you have show non-authorized ON,
  • and it should show in all listings too

the purpose is to limit information displayed of it and do not display fulltext anywhere

Question so does the blog layout show fulltext ?
if it does it is another bug / issue that needs to be fixed

@hardiktailored
Copy link

@ggppdk Yes, showing full text of article in category layouts.


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11290.

@ggppdk
Copy link
Contributor Author

ggppdk commented Aug 4, 2016

@ggppdk Yes, showing full text of article in category layouts.

hhmm i think i can update this PR for this case too,
still it works correctly, if you have introtext (as you are supposed to do)

Also someone needs to update language string of the parameter:
unauthorized links

It has none info about the fact that articles need to have an intro text

@ghost
Copy link

ghost commented Jan 9, 2017

@ggppdk ist this PR updated, should PR be tested?

@ggppdk
Copy link
Contributor Author

ggppdk commented Jan 9, 2017

This PR should be still be a valid fix, but it fixes only article view
it does not fix similar case in category view (i have not updated this PR to include category view)

@ghost
Copy link

ghost commented Jan 10, 2017

I have tested this item ✅ successfully on 0624124

1. Bug verified: Article without Read more and Access Registered all Article Text is shown in Article View.
2. Applied Patch got redirected to login. Logged in as registered User Article shows full Text.
3. After Article-Access set to Special and logged in as registered User got Error: You are not authorised to view this resource.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/11290.

@coolcat-creations
Copy link
Contributor

tested successfully, however the Messages differ from each other:
image
image
One time it´s a message and one time it´s an Error.

@coolcat-creations
Copy link
Contributor

I think at the login redirect it should be a message like "You have to login to have access to the ressource"

@coolcat-creations
Copy link
Contributor

I have tested this item ✅ successfully on 0624124

See my comments to the test


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/11290.

@jeckodevelopment jeckodevelopment added this to the Joomla 3.7.0 milestone Feb 6, 2017
@jeckodevelopment
Copy link
Member

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/11290.

@joomla-cms-bot joomla-cms-bot removed this from the Joomla 3.7.0 milestone Feb 6, 2017
@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Feb 6, 2017
@jeckodevelopment jeckodevelopment added this to the Joomla 3.7.0 milestone Feb 6, 2017
@rdeutz rdeutz merged commit 72c59e3 into joomla:staging Feb 8, 2017
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Feb 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants