User tests: Successful: Unsuccessful:
This pull request adds a new feature called shareable draft content to Joomla. The goal
of this feature is to provide the functionality of generating links that can be shared
so users with this link can preview articles that have not yet been published.
The shareable links are implemented in several views of the content area:
The articles list has a new button called Share draft. This button generates one link per selected article.
The article editing page has a new button called Share draft. This button generates a link for the current article and
shows the generated link on the screen.
The shared drafts list lists all the created shared links. Here the shared links can be managed. The article titles link
to the article editing page if the user has permission to edit the given article.
The article editing page has a new button called Share draft. This button generates a link for the current article and
shows the generated link on the screen.
When a link is generated for the first time, a 16-character token will be created for that specific article. This token
is unique for each article and allows the user with the URL and token to access an unpublished article.
A token cannot be directly regenerated for an article as an existing token will always be returned. To create a new
token, the existing shared link needs to be deleted first from the Shared Drafts view.
By default a regular URL will be created.
In case both SEF and the Redirect Manager are enabled, a SEF URL will be created so an easier to read URL can be used. The
link is generated based on the alias of the article. Once a SEF URL is generated, this will be added to the Redirect
Manager table so it can be used to redirect the user to the correct article.
The following instructions are based on regular URLs, make sure SEF is turned off.
Finally tagging @puneet0191 @chrisdavenport
Status | New | ⇒ | Pending |
Milestone |
Added: |
Labels |
Added:
?
?
?
|
Category | ⇒ | SQL Administration Components Postgresql MS SQL Language & Strings Modules Front End Installation JavaScript |
The token is not secure there is NO validation to check that the token is valid for that article ANY token that exists will work - this is a big red flag
The shareable draft link still works even if the article has been trashed
Final comments
1. is there any point in generating a link when the item is published
2. I thought the whole point of this was to be able to view and share a preview of the content item on the front end but this doesn't really do that as it will only ever generate a preview using the default template and the modules set to be published on all pages
I don't really get the point of this feature.
My understanding is that this generated preview link would only be shared with a few other users. But if it is only a few users, why not use the existing ACL and create a special access level (eg "preview") and put those few users in a usergroup which is allowed to see that access level?
Would achieve the same without needing any code at all, and especially without introducing potential security issues because we bypass the ACL rules.
@mbabker Thank you for your feedback.
@brianteeman The check if something is selected on the articles list has been fixed.
there is NO validation to check that the token is valid for that article
Good point, that is fixed.
The shareable draft link still works even if the article has been trashed
A yes, I will look into that. The same goes for deleting an article.
is there any point in generating a link when the item is published
There is no use for generating a link for a published item.
I thought the whole point of this was to be able to view and share a preview of the content item on the front end but this doesn't really do that as it will only ever generate a preview using the default template and the modules set to be published on all pages
I am not sure I get what you are saying. You can view the content item on the front-end as if it were published. So you get to see how it looks like once it is published. Although in most cases I guess it is more about the content of the item itself than how it looks like. Can you explain the question a bit further?
I don't really get the point of this feature.
You could have told us this GSOC project is useless before GSOC.
My understanding is that this generated preview link would only be shared with a few other users.
Not completely, it would be shared with a few other people. These people don't necessarily have to be users.
But if it is only a few users, why not use the existing ACL
Because they are not users.
Would achieve the same without needing any code at all
It's quite cumbersome and doesn't work for those people who are not users.
and especially without introducing potential security issues because we bypass the ACL rules.
Can you elaborate which security issues are being introduced?
@andrepereiradasilva Thanks, I fixed the name.
I am not sure I get what you are saying. You can view the content item on the front-end as if it were published. So you get to see how it looks like once it is published. Although in most cases I guess it is more about the content of the item itself than how it looks like. Can you explain the question a bit further?
My point was that the preview is only with the default template and any modules set to be displayed on all pages. So for example if your site is setup with templateA as the default and templateB for the content pages the preview will be with templateA. Another example would be that if all your content pages have modules in the left and right columns but they are not for ALL pages then your preview would be full width and not with any left and right columns.
At the end of the day I guess it depends on what the aims of the PR is. If it is to show just the content item to someone who doesnt have access for them to approve then it is ok. But if it is to show a real preview of how that content will be displayed then it probably doesnt achieve that.
I hope that explains it better
Can you elaborate which security issues are being introduced?
That was the token checking issue I believe
I can confirm that the token is now checked
Further to my comment about the generated link and templates and modules. I guess the same (and bigger) problem is that the link takes no account of the language of the content either.
I can see how in its current method/workflow it might be useful for some people but as it is I dont see it as something suitable for the core.
You could have told us this GSOC project is useless before GSOC.
I could have, if I knew about that project, which I didn't.
Can you elaborate which security issues are being introduced?
You are creating access to articles using a simple token. That by itself is a security issue already. Tokens can be bruteforced. Of course it depends on length how long it takes, but it is possible and there is nothing preventing it (like invalidating a token on a failed request).
On a more basic level you are bypassing the whole ACL layer with that token. No matter what we set in the ACL, if that token exists and is known (either through bruteforcing or being shared/leaked), access is given.
I do confirm that, when using multilanguage, this feature is totally broken.
I personally think that is a nice feature. The security issue may come if "disclosing the information of an article before being published can be a issue", but in many sites that I know made in Joomla the information is not that important to worth creating a ACL "Preview" special access level and having to tell the user to register. I think is just one more feature for the Joomla admins to use at their own discretion, this feature exists in other known applications like google docs using tokens.
Still I understand the concerns of bypassing the ACL, there may be a way to creative ways to solve it. I can't think now on any.
Just for your information, there's a vote about this PR within Production Leadership Team.
I'll update this topic as soon as I'll have the result of the vote.
Thank you all for your contribution.
@jeckodevelopment So what's the vote from the Production Leadership Team regarding this PR? I'm willing to test this, but first would like to hear about if this is something that can be added into Joomla.
Hello @JoshuaLewis , voting process has not been completed since PLT started a discussion and review of this PR. I'll share the updates as soon as possible. Thank you for the reminder.
Milestone |
Removed: |
Labels |
Removed:
?
|
It is no where near ready for merging. There is no support for multilanguage and / or the acl
@brianteeman I will look at the multi-lingual issue first. As for the ACL, please explain how you think this should work. Keep in mind that the idea is that anybody with the proper link can access the article.
I dont see why -again- we are creating new db tables when the existing content tables could be modified to hold the sharetoken
While I believe the overall concept is "ok" the currently used implementation is fundamentally flawed, I have the following comments too...
Why is the token only 16 chars?
I dont see why -again- we are creating new db tables when the existing content tables could be modified to hold the sharetoken
There is nothing stopping bots from indexing this content...
There is no expiration or time limit on a token being used....
Why are we sharing by only a token for anonymous access, why can we not restrict this further to a "user group membership & token" or even do away from using a token and allow users to see shareable content by membership of a group - therefore reusing the existing ACL in Joomla (totally more secure!)?
Use of the redirects table as well, what happens if a shareable url is deleted, yet the redirect remains?
I don't understand why all this is a separate extension/feature when it could be implemented as just another "state" in the array published, unpublished, trashed, etc... and then locked down by ACL group membership, this way when the content is loaded on the frontend you would also get to see it "in context" with the correct url and modules and templates etc... Its menu items could also then be shared and seen in context.
If you really need shareable "CONTENT" accessed by JUST a sharetoken then it should be a blank page EXCEPT for the content, with minimal styling, to show the person using the token the CONTENT and nothing more, no context etc...
Testing instructions only give
"The following instructions are based on regular URLs, make sure SEF is turned off."
and
"Enable SEF URLs & Redirects plugin is enabled"
What happens if SEF URLs are enabled & Redirects is DISABLED???
If my token is
2aCXKfntOMP7AAV2
but in the url I use something like
2aCXKfntO%27"%27"%27"%27"MP7AAV2
(which is some single and double quotes) then the shared content STILL LOADS ... this just seems plain wrong, I know that the value is filtered, but surely it should first be validated then filtered, then validated again ? I think though this is a bigger Joomla issue not just with this new stuff.
It just doesnt seem right that I can provide a string that is not being equated like !== and still gain access to content.
2aCXKfntOMP7AAV2
!== 2aCXKfntO%27"%27"%27"%27"MP7AAV2
Given that I have not looked at this in months, I don't have an answer for every question at this point.
@zero-24 Thanks for the feedback, will process it.
@PhilETaylor
Why is the token only 16 chars?
Should it be 160? 1600? There is no science behind this, just that 16 seemed good enough. If you tell me how it should be, I can update the code.
I dont see why -again- we are creating new db tables when the existing content tables could be modified to hold the sharetoken
The reason for this was that in the original proposal there was more data stored in this table, like the number of views, a title for the share and when it was shared. Agree, we can add those 2 columns for the token and share date to the main content table.
There is nothing stopping bots from indexing this content...
How would that happen? They generate the tokens?
There is no expiration or time limit on a token being used....
Indeed, was not part of the original proposal either.
or even do away from using a token and allow users to see shareable content by membership of a group
. and then locked down by ACL group membership
This way you can only let people with an account on the site view the content. How would you deal with people that don't have an account?
If you really need shareable "CONTENT" accessed by JUST a sharetoken then it should be a blank page EXCEPT for the content, with minimal styling, to show the person using the token the CONTENT and nothing more, no context etc...
What is wrong with showing content?
What happens if SEF URLs are enabled & Redirects is DISABLED???
Did you try? I have no clue :)
It just doesnt seem right that I can provide a string that is not being equated like !== and still gain access to content.
We can do a second check on the content after it has been retrieved from the database. I have no problems with that.
This way you can only let people with an account on the site view the content. How would you deal with people that don't have an account?
It doesnt seem unreasonable that if you want to view my unpublished content, that you should have an account by which I can assign ACL to for you to access the content - draft or published...
Like I said before, I think the whole, allow unauthenticated, unregistered users complete unlogged access to a private resource by accessing a url a flawed design.
If Joomla allowed anyone to access content with nothing but a url - this would be considered a serious security issue. I dont see the difference if the url is provided by the admin or guessed by a hacker.
There is nothing stopping bots from indexing this content...
How would that happen? They generate the tokens?
It happens that there is a lot of stuff in google that comes from unknown sources - even browser extensions would have access to these urls, Ive seen that before...
If Joomla allowed anyone to access content with nothing but a url
That is how visitors access my site, they don't need to login to see the content.
@roland-d if you are going to quote me out of context then I'll leave.
Maybe I should have said:
If Joomla allowed anyone to access UNPUBLISHED content with nothing but a url - this would be considered a serious security issue. I dont see the difference if the url is provided by the admin or guessed by a hacker.
There is no use in continuing this PR unless we decide whether or not we allow unregistered users to see content via a link.
If you need to be registered, the whole sharelink is obsolete, as you pointed out, we can then use the ACL we have.
@chrisdavenport What is your input on this?
My two cents...
There are use cases where this type of workflow is indeed valid. So conceptually it's not a terrible idea. Whether the implementation is up-to-par is up to others who've spent time with this code to decide, I've just been doing drive-by looks.
Is it possible with the sharing content to share something that is member
only. The site i am building right now could use a sharing function for
approval but the live content would be set to be member only. So it would
be a massive issue for this site if the url was leaked and it could only be
shared if it was set to ar a minimum the acl of the live site.
On 6 Nov 2016 5:58 p.m., "Michael Babker" notifications@github.com wrote:
My two cents...
There are use cases where this type of workflow is indeed valid. So
conceptually it's not a terrible idea. Whether the implementation is
up-to-par is up to others who've spent time with this code to decide, I've
just been doing drive-by looks.—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#12009 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABPH8QEhooQlS2QzyGCXmKSogTbxDDN6ks5q7gccgaJpZM4J6ABJ
.
There is no way to globally enable/disable this feature.
At the moment the only way to disable access to existing links is to delete them from shareddrafts view... there is no global way to deny access to all those links
There is no logging on when those links are accessed
There is no expiration on the links by date or number of views
An article that is set for "Access: Super Admins or Registered" can NOT be viewed by Anon with the shareable link - thus defeating the purpose of the feature.
There is no restriction at all on the links
I also dont like that "eM
If you are happy with all the above... as a site owner, then great... but it doesnt sound like something thats secure or should be enabled as a feature by default.
The idea of a shareable link is to get around the issue of having to register people before they can look at draft content. This is a feature that has been requested many times over the years and has already been implemented in some other CMSs.
I haven't looked at the code so I can't comment on specifics about how it has been implemented, but based on the comments above...
@brianteeman I don't see any reason why we couldn't restrict access to a specific viewing access group as long as "Guest" is not a disallowed setting.
Right now you can only share public content which makes it unusable for any
site that uses the acl.
I also note that previously I commented that this doesn't support
multilingual
So I updated this PR with 2 small changes. First I increased the length to 32 characters, if this needs to be longer, just let me know. Second I implemented strict checking on the token as pointed out by @PhilETaylor earlier.
@brianteeman I have been thinking about how this should work in a multi-lingual site. As it is now, each article would have it's own token. I guess the idea is that an article would be able to change based on the language switcher?
Iirc and I am not at my PC the problem was the language of the site that
was used on the shared item was not necessarily the language of the article
Just applied the patch
Made sure the redirect plugin was enabled
Selected two article in the article list and clicked "share draft"
Message
COM_CONTENT_DRAFT_LINK_ERROR
Created a new article and saved it and the "share draft" button appears
Click on the button
Error
COM_CONTENT_DRAFT_LINK_ERROR
@brianteeman I see what I missed in the instructions, that is that the database table needs to be created. I think that Fix on the Database page should do that.
I only want to share another way how it could be implemented.
client
with ACL access to my shared article(s).Create somewhere link to auto login client
with expiration date.
Ex http://example.com/autologin=xxxxxxxxxxxxxx
where
xxxxxxxxxxxxxx
will contain crypted(username + hash of password + expired date + maybe crc)
Then append redirection parameter:
`http://example.com/autologin=xxxxxxxxxxxxxx&return=edit_article_link
client
and redirected to sharable content.client
and you have full access to client
account.I'm quite sure you could actually do that with a system plugin.
Finally I do not believe that this PR even achieves the aim of sharing the content. If I wanted/needed this feature then I would want to see the content in exactly the way it is displayed on the site. That means that it must be displayed with the correct modules and template and article options. This can only be done by using the ItemID and this PR does not use any itemid at all. (might explain point 3 above)
Category | SQL Administration Components Postgresql MS SQL Language & Strings Modules Front End Installation JavaScript | ⇒ | SQL Administration com_admin Postgresql MS SQL com_content Language & Strings Modules Front End Installation JavaScript Components |
Next problem is that the url that is being generated is without any token
When the SEF URLs are enabled, a nice URL is generated and the redirect manager is being used. Only if SEF and Redirect manager are enabled this will be used, otherwise you get a regular URL.
The shared draft link is still valid even after the article is trashed
That has been fixed.
Still breaks the layout of the toolbar on a reasonably large screen (anything less that 1392px)
Can you post a screenshot of that? I mean, the buttons are just part of the normal Joomla toolbar.
Doesnt work at all for a multilingual site. The shared draft gives a 404 for any content that is not either lang=all or lang-en-gb
That should be fixed as well now.
Create a shared link for an article that already has a link and the message says it has been created but if course it hasnt as it already exists
Any suggestion what the text should be?
Create a shared link for an article that already has a link and the message says it has been created but if course it hasnt as it already exists
Any suggestion what the text should be?
A draft link for this item already exists
Perhaps
Missing string from the ini
COM_CONTENT_DRAFT_LINK_ERROR
The shared draft link is still valid even after the article is trashed
That has been fixed.
Confirmed this is working
I see that you still dont address what for me is the most critical flaw in this entire component -the lack if itemid so you dont see the correct modules
Finally I do not believe that this PR even achieves the aim of sharing the content. If I wanted/needed this feature then I would want to see the content in exactly the way it is displayed on the site. That means that it must be displayed with the correct modules and template and article options. This can only be done by using the ItemID and this PR does not use any itemid at all. (might explain point 3 above)
The Itemid has been fixed. The code will first look for a menu item linked to the article, if not found, it will check the category the item is linked to. If there is item id found, it will continue searching the parent categories until one is found or until the top of the tree has been reached. Not sure if this is the correct approach or if something is missing, if so I would like to hear it so it can be fixed.
The missing language string has been fixed.
As for the layout issue of the toolbar, this is not related to this PR, you will get the same issue without the extra button. The issue is being discussed in #12789
Thank you for looking at those issues. I will test tomorrow
Sorry - still doesnt work in multilingual - see video
Status | Pending | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2017-01-15 08:03:35 |
Closed_By | ⇒ | roland-d |
:-(
I closed it due to lack of interest.
I was holding back from testing this PR until Brian's suggestion about the multilingual issue was resolved. Otherwise I would gladly test this PR.
Status | Closed | ⇒ | New |
Closed_Date | 2017-01-15 08:03:35 | ⇒ | |
Closed_By | roland-d | ⇒ |
Status | New | ⇒ | Pending |
I will give it another try.
As the comments show - there is not a lack of "interest"
The problem is the implementation of the feature has not been completed, has issues, @brianteeman has been banned from contributing for a period of time so cannot follow up on the comments, and no one else has taken time to provide the additional development work or feedback.
This branch now conflicts and so also needs cleaning up
Also has the PLT actually made a decision that this can be eventually merged? If they dont think its a "good feature for Joomla" then it will never be merged anyway - you might want to seek that "approval" before wasting much more time on it.
@PhilETaylor There is approval from the PLT to get this merged.
A few people is not really a lot of interest is there. Brian was the only one really testing and providing feedback. The comments are for from showing interest but rather saying this can be done better or otherwise.
Cleaning up the conflicts is not going to be an issue.
Well as with all things Joomla, the PLT have the final say - rightly or wrongly - about the method of implementation.
Brian cannot comment at the moment, but Im sure if he could, he would.
I will update this PR and follow up on Brians last comment. We will see who is going to test then. We are going to need at least 2 testers anyway and preferably a few more with something like this.
I will definitely be a tester once this PR gets updated. I have a decent amount of confidence in getting another tester from this circle of Joomlaers. Likely 3 testers.
@JoshuaLewis would test too.
@JoshuaLewis @franz-wohlkoenig I have cleaned up some code, fixed the multi-language issue and some other minor tweaks.
The biggest change is a change in the DB structure. Please make sure your DB structure is up-to-date as laid out here: https://github.com/joomla/joomla-cms/pull/12009/files#diff-c69a128c6c4206461444b099861fa310
Perhaps the Fix Database may work, I am not sure.
Let me know if you have any questions.
Hello @franz-wohlkoenig I am not sure I fully understand what you say is wrong but I understand that when you click the Share button on the article edit page you are redirected to the control panel?
I did find an issue with the front-end not showing the article but that has been resolved with the last commit.
If Redirect-Plugin is disabled, all work well, no SEF-Url.
If Redirect-Plugin is enabled, the SEF-Url don't work as expected, cause i'm redirected to Home-Mainmenu.
Example:
index.php/category-list-category-list/test1
: Edit Article "test1", shared Link is __/index.php/test1
> redirect on Default-Page.
@franz-wohlkoenig I have tested with the Redirect plugin as well and I get a normal SEF URL. You get a URL with two underscores: __/index.php/test1 ? That I have never seen.
Can you find this URL in the list of redirect URLs? How is it stored there? The URL that is generated by the shared draft should be redirected to the non-SEF URL stored in the Redirect list.
@franz-wohlkoenig The screenshots explain it. The share draft won't generate the link that you will get from the menu item. The index.php/article-by-author is the expected URL, as the New URL has the correct Itemid, it should show the same layout as when you open the menu link.
Is your issue that the share draft URL is not the same as the menu URL? Is the New URL not opened when you access the share draft URL?
When you open the SEF URL, you are redirected to the homepage correct? If you open the New URL, do you get the correct page in that case?
You have the latest updates from this PR on your test system?
Is your issue that the share draft URL is not the same as the menu URL? Is the New URL not opened when you access the share draft URL?
Exact, new Url is not openend.
When you open the SEF URL, you are redirected to the homepage correct? If you open the New URL, do you get the correct page in that case?
Corect, User is redirected to Default-Homepage (Start). Got correct Page if Plugin "Redirect" is disabled.
You have the latest updates from this PR on your test system?
I load every Morning default-staging down and install them. Maybe if in the Meantime ar changes i don't get them.
No Difference if open Article in "Blog view" or "Menu: Single Article" – Plugin generates Url index.php/name-of-article
(test on latest staging).
Tested the latest version in mono-lingual web site
Created a category and a menu item to display that category
Created an article and saved in the category above
Created a shared draft link
As you can see there is NO itemid in the link
The article displays on the home page it should be displayed in the menu item created for that category
See this video https://www.dropbox.com/s/7ov048kbpfk0xev/roland.mp4?dl=0
PS: as seen on Brian's twitter!
Hint: Modern routing is enabled on Article, Contact, News and Users.
I'd love to see this eventually connected to the Version system in the future, allowing you to create a draft version and edit any number of saved drafts and generate these preview links for any draft.
I have tested this item
Pardon the late testing. Successfully works, thanks Roland for the fixes. :-D
@franz-wohlkoenig I tested with the modern router with success. Am I missing something? As for the menu item inclusion in the URL, indeed it would be ideal if we can get the draft to use the menu item for display/permission reasons.
@JoshuaLewis tested with legacy router, multilingual Site.
Works without SEF.
If
Search Engine Friendly URLs
on Yes
(no Difference if Use URL Rewriting
is enabled or not)All
or en-GB
got in Front- and Backend: 404 Category not found
Joomla! 3.7.0-beta3
macOS Sierra, 10.12.3
Firefox 51.0.1
@franz-wohlkoenig I was able to reproduce your issue after enabling the Redirects plugin. I assume you have this turned on as well. Is that correct?
Correct, Redirect-Plugin was always on.
@franz-wohlkoenig I have narrowed down the issue to the System - Language Filter with the option Remove URL Language Code turned on. Once I turn this off, the failing URL now does load correctly. So I need to check for this setting when generating the URL.
you give me a go for testing.
@franz-wohlkoenig I believe it is fixed now with the last commit. Can you give it a try? Thank you,
@brianpeat That is one of the things we can think about further developing this feature but for now that is out-of-scope.
As a general remark regarding Itemid. It is quite tricky to find the correct Itemid as the URL can change based on layout or tag setting. Perhaps even more settings. So there will be cases when the Itemid is not found.
@franz-wohlkoenig A few questions:
Which language is default in your site?
en-GB
Is the Remove Language Code set to Yes?
Plugin Language Filter: Remove URL Language Code: No
(same Result if set to Yes
).
Which language is the article?
Which URL do you get?
Do you see the URL in the Redirect manager?
No. Redirect Plugin and option Collect URLs
is enabled.
Which URL shows up in the Redirect manager?
What should the front URL be?
@franz-wohlkoenig What I am trying to achieve is the same setup as yours.
So I created an article in the backend and now have a URL generated like this:
http://localhost/ml/index.php/en/test-single-article-link-creation-in-back-end
Do you see the URL in the Redirect manager?
No. Redirect Plugin and option Collect URLs is enabled.
That is not true because in the screenshot I see the URL is there, the first entry.
What should the front URL be?
url:
index.php/en/blog/8-category-en-gb/7-test-single-article-link-creation-in-front-end
Share url:
index.php/en/test-single-article-link-creation-in-front-end
What I actually meant with the front URL is the non-SEF URL, not the URL created by editing an article in the front-end.
Let's get back to the back-end URL.
You also have the URL index.php/en/test-single-article-link-creation-in-back-end to show the article in the front-end. This will redirect you to index.php?option=com_content&view=article&id=76&token=yourtoken&lang=en
So that all looks good to me. Only question I have actually for now is, does the http://localhost:8080/J3/index.php/en/test-single-article-link-creation-in-back-end open the article?
That is not true because in the screenshot I see the URL is there, the first entry.
My fault. Forget to delete info.
So that all looks good to me. Only question I have actually for now is, does the http://localhost:8080/J3/index.php/en/test-single-article-link-creation-in-back-end open the article?
so is this dead now? or is it actually still a proposed new feature?
It has conflicts with the base to fix too.
I am going to close this as I can't find the desire to pursue this any further.
Status | Pending | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2017-07-07 19:19:21 |
Closed_By | ⇒ | roland-d |
There needs to be a Javascript warning when you select the Share Draft button from the article manager view when you have not selected any articles - that would be more consistent with all the other toolbar items than displaying a message
0 draft links saved successfully