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

ACL for Content Language and dropdown #30

Closed
wants to merge 37 commits into from

Conversation

infograf768
Copy link
Owner

Install staging as multilingual with 3 languages with the new asset_id in the _languages table.
Create a subgroup of administrator and add a user to that group.

Edit any Content Language new Tab Permissions.
Set the newly created group to Denied for the "JACTION_PERMISSION"

Save. Now login with the new user and edit an article for example;
Look at the language dropdown: the Content Language which is denied to this group will not display.

@infograf768
Copy link
Owner Author

For info, the other Actions in the new Permissions are not used at all for the moment.

protected static $assets = null;

/**
* Get all language asset permissions
*

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since 3.6.0

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this will only go for 3.7.0. now.

@andrepereiradasilva
Copy link

@infograf768 i notice that by default the permission is set to "Not allowed".
This needs to be by default "Allowed". On upgrade and when creating a new content language.

image

@andrepereiradasilva
Copy link

Also you can't remove the langauge from the dropboxes, we need to have it there, but disabled.

For instance, if i have a module in portuguese even if i can't change the language to portuguese i need to know that it is portuguese and not fallback to "All" when saving.

@andrepereiradasilva
Copy link

andrepereiradasilva commented Jun 7, 2016

For info, the other Actions in the new Permissions are not used at all for the moment.

i can work on this part and make PR here and when this is ready (probably for 3.7.0) we send it to main joomla repo, ok?

@andrepereiradasilva
Copy link

merge #31 for first part of ACL in languages list view

@@ -8,4 +8,12 @@
<action name="core.edit" title="JACTION_EDIT" description="JACTION_EDIT_COMPONENT_DESC" />
<action name="core.edit.state" title="JACTION_EDITSTATE" description="JACTION_EDITSTATE_COMPONENT_DESC" />
</section>
<section name="language">
<action name="core.permission" title="JACTION_PERMISSION" description="JACTION_PERMISSION_LANGUAGE_DESC" />
<action name="core.manage" title="JACTION_MANAGE" description="JACTION_MANAGE_COMPONENT_DESC" />

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the language itself doesn't need the core.manage acl.
I also would move the core.permission to be the last one.

@andrepereiradasilva
Copy link

merge #32 for second part of ACL in languages list view (do not allow to delete without permissions)

@infograf768
Copy link
Owner Author

@andrepereiradasilva @robclayburn

In fact, the more I think of it, the more we should limit the new ACL in back-end to the associations (new component and present implementation of associations per item). I.e. not modify for now the contentlanguage field type but create a new field type that would be used only for the new component.

As for the existing associations implementation, it will need a specific coding in the preprocessForm() for each core component.

The reason is that a back-end user who is not permitted to choose a specific content language but is allowed to edit items will not be able to keep the existing tagged language if he has no permission for that language as the field for all items is the default contentlanguage field type.

To implement this, it would need a lot of work which is not for this GSOC project.

Now looking for the implementation of permissions in our present item per item associations.

@infograf768
Copy link
Owner Author

@andrepereiradasilva
I merged both your PRs

@infograf768
Copy link
Owner Author

Modified access.xml as you suggest. I do not think we need core.permission for the component.

@infograf768
Copy link
Owner Author

For contenlanguage type, let's wait we agree on my comment above as we may need a new type instead of modifying the existing one.

@andrepereiradasilva
Copy link

Modified access.xml as you suggest. I do not think we need core.permission for the component.

I mean as fallback for "Undefined" (when contentlanguage is deleted and there are items associted to it) language.

@infograf768
Copy link
Owner Author

This needs to be by default "Allowed". On upgrade and when creating a new content language.

I would love it but I have no idea how to.
Rob provided a fallback to true when the content language is used without saving, to be B/C

// Default to true for old content languages which have not had their acl saved.
            $canDoAssociations = !is_null($asset) ? $user->authorise('core.permission', $asset->name) : true;

But I have no idea how to set it to Allowed as default if a user edit a content language or create one without dealing with its permissions.
See https://issues.joomla.org/tracker/joomla-cms/7517

@infograf768
Copy link
Owner Author

[08/06/16 12:39:17] Thomas Hunziker: You can create the ACL the opposite way: "Block Language". When it's "inherited/disabled" it will show. :)
[08/06/16 12:39:28] Thomas Hunziker: But it's a hack :p
[08/06/16 12:39:51] infograf768: ah, fun
[08/06/16 12:39:57] infograf768: indeed
[08/06/16 12:40:18] infograf768: good idea

But Bakual also says that he would not personnaly add this to core...

@andrepereiradasilva
Copy link

But Bakual also says that he would not personnaly add this to core...

why not? add ACL in a component that doesn't have ACL now shouldn't be part of the core?

@@ -439,7 +440,10 @@ protected function preprocessForm(JForm $form, $data, $group = 'content')

foreach ($languages as $tag => $language)
{
if (empty($data->language) || $tag != $data->language)
$permission = $user->authorise('language.permission', 'com_languages.language.' . (int) $language->lang_id);
$canDoAssociations = !is_null($permission) ? $permission : true;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if is null i think it meand not allowed so it can't.
So, it should be just this line:

$canDoAssociations = $user->authorise('language.permission', 'com_languages.language.' . (int) $language->lang_id);

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be B/C, it had to be true by default, as Rob proposed. If your last ACL patch goes in, we can test what it gives then and decide.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my patch that sets "Allowed" as default is already integrated into this one :)
so IMO this doesn't make sense anymore

@andrepereiradasilva
Copy link

@infograf768 where we are on this.
we will need this merged for associations gsoc project

@infograf768
Copy link
Owner Author

@andrepereiradasilva

As I said above, we need to create a new field type and that field type should normally be placed in com_associations. Therefore we have to wait for com_associations to have a bit more than what it has right now...

@andrepereiradasilva
Copy link

ok then let's wait

@infograf768
Copy link
Owner Author

infograf768 commented Jul 23, 2016

This is what I proposed in chat:

There is a solution for the time to come.
We do not change the JHTML contentlanguage (which I was doing here), neither do we change the contentlanguage field itself for now.

What we can do is create a new JHTML assoclanguage in com_associations/helpers/html/ and a new field type assoclanguage in com_associations/models/fields/ loading the new JHTML.
then we use the new field in filter_associations.xml

For the side by side view, we modify itemlanguage.php to take into account the new permission.

This modified PR would be included when we propose com_associations to PLT

@infograf768
Copy link
Owner Author

I have prepared the 2 necessary files for com_associations + some changes for filter_associations.xml and itemlanguage.php

@infograf768
Copy link
Owner Author

Found a possible issue in this PR concerning editing associations in the existing interface (not in com_associations).
I think that if one has access to edit an item, one would be able to choose an associated item, but would only be able to edit it if permission is granted for that content language.

The code we have now does not let choose an associated item.

Therefore the code would be for each component concerned:

                if (empty($data->language) || $tag != $data->language)
                {
                    $add = true;
                    $field = $fieldset->addChild('field');
                    $field->addAttribute('name', $tag);
                    $field->addAttribute('type', 'modal_article');
                    $field->addAttribute('language', $tag);
                    $field->addAttribute('label', $language->title);
                    $field->addAttribute('translate_label', 'false');

                    // Do we have permission to edit this associated item?
                    if ($canDoAssociations)
                    {
                        $field->addAttribute('edit', 'true');
                    }
                    $field->addAttribute('clear', 'true');
                }

@andrepereiradasilva
What do you think? We should take a decision.

(for menu items, as we do not have a modal, we can't edit anyway.)

@andrepereiradasilva
Copy link

i'm not yet confortable in understanding the complete solution.
I think we need to put it all in one text so we can decide better to check possible implications.

@infograf768
Copy link
Owner Author

@andrepereiradasilva
@roland-d

In fact, night having passed over, what I just wrote above yesterday makes no sense.
Edit should not be depending on Content Language permissions.

After some thought, here is what I came to:

Although we should keep what is done here concerning com_languages permissions (i.e. creating, editing, etc. the Content Languages themselves in the Languages Manager, we should reconsider the part concerning the use of these permissions for associations.

A user should be able to Associate items or not, period. The permissions for that should be in com_associations, used for that component (access or not to the component) but also everywhere where we can associate, i.e. in each component using associations.
In that case, we do not touch at all to the contentlanguage field (in fact JHtmlContentLanguage)

This means that the Associations Tab in general (in each component) would display fully ONLY when the user has permission (fetched from com_associations). Modification to do in the edit_associations.php file.

In these files we do have now:

if ($this->item->id != 0 && $this->item->language != '*')
{
    echo JLayoutHelper::render('joomla.edit.associations', $this);
}
else
{
    echo '<div class="alert alert-info">' . JText::_('JGLOBAL_ASSOC_NOT_POSSIBLE') . '</div>';
}

We would fetch the user, check if he has permission to modify associations and, if not, display a new message. The alert would come before the other one.

For the items themselves, their modification should only depend on the component concerned permissions (edit, create, edit_own, delete), as it is today.
Evidently deleting an item would also, as of today, de-associate it.

NB: Being able to assign/modify a content language to an item, as I wrote in #30 (comment) is a too much complex implementation for now and should be considered for 4.0.

@roland-d
Copy link

roland-d commented Jul 26, 2016

@infograf768

A user should be able to Associate items or not, period.

I believe that should be the case. He/She can't manage the item without proper permissions but still should be able to associate it.

infograf768 pushed a commit that referenced this pull request Jun 3, 2017
* codestyle

* code style

* codestyle

* codestyle

* codestyle

* thanks @wojsmol

* corrections - thanks @Quy

* corrections - thanks @Quy

* oops

* make @Quy happy

* Update article.xml

* Remove space

* Update config.xml (#14)

* Update filter.xml (#15)

* Update config.xml (#16)

* Update profile.xml (#17)

* Update application.xml (#18)

* Update article.xml (#19)

* Update filter_articles.xml (#20)

* Update config.xml (#24)

* Update config.xml (#23)

* Update filter_fields.xml (#22)

* Update filter_featured.xml (#21)

* Update override.xml (#25)

* Update config.xml

* Update config.xml (#26)

* Update itemadmin_alias.xml (#30)

* Update itemadmin.xml (#29)

* Update item.xml (#27)

* Update item_alias.xml (#28)

* Update itemadmin_url.xml (#31)

* Update module.xml (#32)

* Update plugin.xml (#33)

* Update config.xml (#34)

* Update link.xml (#35)

* Update config.xml (#36)

* Update style.xml (#38)

* Update config.xml (#37)

* Update note.xml (#42)

* Update group.xml (#41)

* Update filter_debuggroup.xml (#40)

* Update config.xml (#39)

* corrections for @andrepereiradasilva

* gotya
@sanderpotjer
Copy link

@infograf768 is this still the latest attempt to get ACL for content languages in Joomla, or are there other / more recent PR's?

infograf768 pushed a commit that referenced this pull request Sep 6, 2018
* Load correct core files of override files (#2)

Start implements loadcorefile() in administrator/components/com_templates/Model/TemplateModel.php

* CS (#3) Coding Standards

* codingstandards

* codingstandards (#4)

* Test (#6)

Phase 2 (2 part) Mechanism to find correct core file and implementation.

* Remove Notice: Only available for html-folder

* Remove Warning if core file not found (#11)

Thanks.
So one part of the issue joomla-projects/gsoc18_override_management#12 is done.

* Implement the diff view in template manager 

Implement the diff view in template manager

* coding standard (#17)

* fix diff (#18) Fix bug in path in case of administrator template override.

Fix bug in path in case of administrator template override.

* Notification after update and TEST (#16)

Find changed files of overridden files and show message.

* coding standard (#21)

* correction

* correction (#26)

* Correcthtmlpath (#27)

* correction

* change oldhtml to newhtml

* List of updated override files. (#30)

* addcss (#34)

* Final Product  (#39)

Core and Diff view
Updated override history list.
Quick icon notification plugin.
Override control plugin.

* save 3 lines :)

* New feature show status. (#47)

show status in com_template view templates

* link

* corrected namespace

* Button to Switch (#35)

* wip add Switcher

* wip style switcher

* wip style switch make inline and change on off text

* wip start with js

* wip js

* wip delete buttons and make js more robust

* wip save to storage

* wip delete old code

* wip

* wip lint

* wip css

* set default value for switcher

* wip make switcher blue

* wip

* wip

* build

* correct names

* create new functions

* fist test code

* use onchange

* undo installer.min.js

* add forgotten new line at the end of css file

* correct align

* correct compare.es6 - only deleted the toggle part

* correct compare.js - only deleted the toggle part

* wip

* reduce timeout

* wrap in funcitons

* wip

* add use strict to both js-files(compare and toggle)

* add the timeout value of 500 again, because 200 are not enought in my case

* use css class 'active' for toggle views

* add strict

* time out for editor

* wip

* improvments use newActive and switch

* correction

* width of switcher-spans

* correct align

* do not use global

* wip

* removed timeouts

* JTEXT to TEXT

* forgotton last line

* deleted duplicated comments

* css fix align

* use unnamed functions in es6

* Sql files for fix database (#50)

* sql files for database fix

* delete space

* Suggestion for displaying Dates in view updates files (#52)

Correct Dates and do not use date of file any more

* Store Date as UTC and show it in server time zone (#57)

* modified and created date are created and stored in UTC

* convert dates for displaying in model

* spar a loop

* normalize timezone in view

* use language constants for dateformat

* JToolbarHelper to ToolbarHelper

* CS

* namespace

* plural

* name

* clean

* text

* fx

* sin

* files

* s

* Suggestion for language strings (joomla#60)

* language strings

* correct typo

* delete media folder plg_quickicon

* add folder plg_quickicon to build/media_src

* delete files in media folder

* Move media folder - System (joomla#66)

* multi

* cs

* delete files in media folder for joomla toolbar (joomla#67)

* Fix button switchers style. (joomla#70)

* button

* CS

* changed uitab.addTab for updated files

* Bring back core.js changes. (joomla#69)

* core.js

* const

* fix

* form

* core

* hound

* CS

* scopr

* grid

* alpha

* cs

* lang

* only override file

* lang

* override lang installer

* Cs

* sub

* Update list of core extensions (joomla#71)

* Language changes (joomla#76)

* update

* Update en-GB.com_templates.ini

* override JLIB_HTML_PUBLISH_ITEM

this is the hover text on the publish icon in the list of files

* Change icon (joomla#74)

change the icon to use an outline for more consistency

* lang

* not core (joomla#75)

* not core

* Update en-GB.plg_installer_override.ini

* namespace

* cs

* Updated files (joomla#82)

* Update default_updated_files.php

* Update en-GB.com_templates.ini

* Update en-GB.com_templates.ini (joomla#81)

* Update en-GB.plg_quickicon_overridecheck.ini (joomla#80)

* Update en-GB.plg_quickicon_overridecheck.ini (joomla#79)

* remove space (joomla#78)

* Update en-GB.plg_quickicon_overridecheck.ini

* Update en-GB.plg_quickicon_overridecheck.sys.ini

* remove hardcoded id

* null get function

* state

* clean

* More changes "core" to "original" (joomla#85)

* cs

* update

* plural
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants