Re: [Freeipa-devel] Automated Fedora update testing

2017-05-03 Thread Petr Vobornik

On 04/29/2017 02:07 AM, Adam Williamson wrote:

Hi folks! I thought this might be of interest to the FreeIPA community,
so I thought I'd write it up here in case anyone missed it elsewhere.





Until recently we ran these tests only on Fedora's nightly development
release distribution composes. Recently, though, we deployed some
enhancements to our openQA setup that let us run tests on Fedora
distribution updates as well, and have the results made visible through
the Fedora update system (Bodhi). The tests are automatically run on
any critical path package, and as of today, they are also run on any
update containing any of a manually-tended list of FreeIPA-related
packages:

389-ds
389-ds-base
bind
bind-dyndb-ldap
certmonger
ding-libs
freeipa
krb5-server
pki-core
sssd
tomcat
cockpit

This means that for any Fedora update containing one of these or any
critical path package, Fedora's openQA FreeIPA tests should run, and
you should see the results in the Fedora update system (Bodhi). You can
see the results in Bodhi by clicking the Automated Updates tab for any
update. For instance, here's a recent 389-ds-base update for Fedora 26:

https://bodhi.fedoraproject.org/updates/FEDORA-2017-15e2a038b2

If you look at the Automated Tests tab, you can see passes for:

update.server_role_deploy_domain_controller
update.realmd_join_cockpit
update.realmd_join_sssd

indicating that this update didn't cause any problems for FreeIPA.
Clicking on any test result will take you to the openQA page for the
test, where you can diagnose failures and so on (explaining how to do
this is a bit beyond the scope of this mail, please do ask me if you're
interested!)


This is really great.



I hope this stuff will help us avoid shipping updates that break
FreeIPA (and other key components). If you have any questions,
concerns, comments, or suggestions, please do ask!
.

Note, if you're interested in the results for the nightly Fedora
distribution composes, an email summary of the results for those is
sent each time they're run to the Fedora test@ and devel@ lists, look
for mails with "compose check report" in the subject. Any time any of
the FreeIPA tests fails, the failure will be listed in the mail (passed
tests are not specifically listed, just a count of them). I usually
keep an eye on those results and analyze failures and file bugs,
though.



Is there a way now to check current state of current Fedoras 
automatically using a script - e.g. avoid parsing mailing list or going 
through runs in OpenQA?


--
Petr Vobornik

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


[Freeipa-devel] "blocker" tag for pull request

2017-04-28 Thread Petr Vobornik

Hi all,

I created "blocker" tag for FreeIPA Git Hub PRs.

It is should be used to mark PRs which solves test blocker or other 
functional blockers - e.g. blocks creation of demo. I.e. should be used 
rather rarely.


I don't like the tag name, but I couldn't find better.

Note: blocker priority in pagure doesn't imply blocker tag in PR. But 
testblocker tag in pagure does. Actually I'm thinking about changing 
Pagure priority names to: "highest, high, medium, low, patchwelcome"


--
Petr Vobornik

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] KDC proxy URI records

2017-04-27 Thread Petr Vobornik

On 04/27/2017 02:19 PM, Christian Heimes wrote:

On 2017-04-27 14:00, Martin Bašti wrote:

I would like to discuss consequences of adding kdc URI records:

1. basically all ipa clients enrolled using autodiscovery will use
kdcproxy instead of KDC on port 88, because URI takes precedence over
SRV in KRB5 client implementation. Are we ok with such a big change?


Does the client also prefer KKDCP if you give the Kerberos 88/UDP and
88/TCP URIs a higher priority than the KKDCP HTTPS URIs?


2. probably client installer must be updated because currently with
CA-full installation it is not working.

ipa-client-install (with autodiscovery) failed on kinit, see KRB5_TRACE
bellow that it refuses self signed certificate


Actually it is not a self-sigend EE certificate. The validation message
is bogus because FreeIPA TLS configuration is slightly buggy. We send
the trust anchor (root CA) although a server should not include its
trust anchor in its ServerHello message. OpenSSL detects an untrusted
root CA in the ServerHello peer chain and emits the message.

If I read the 600 lines (!) function ipaclient.install.client._install
correctly, then ipa-client-install first attempts to negotiate a TGT and
then installs the trust anchor in the global trust store. It should be
enough to reverse the order and inject the trust anchor first.

Christian



By reading this, even if we do the change in client install, I'd rather 
not generate the DNS records in 4.5.1 release and rather make sure that 
everything works during 4.6 development.


The reason is that there might also be something else not working and it 
is better to time test it + the fix would not fix older clients.


If anybody wants to use/try it, then the records can be created manually.
--
Petr Vobornik

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


[Freeipa-devel] [HEADSUP] 389-ds-base-1.3.6.4-1.fc26. and 389-ds-base-1.3.5.17-1.fc25 breaks server installation

2017-04-26 Thread Petr Vobornik
New builds of 389 on F25 and F26 breaks server installation. Or at least 
I think it's 389 issue on f26.


f25
https://bodhi.fedoraproject.org/updates/FEDORA-2017-8bb5a83e04

f26:
https://bodhi.fedoraproject.org/updates/FEDORA-2017-7f0a10c808
https://bugzilla.redhat.com/show_bug.cgi?id=1445776

--
Petr Vobornik


--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] Pagure issue template

2017-04-21 Thread Petr Vobornik

On 04/21/2017 08:49 AM, Standa Laznicka wrote:

On 04/21/2017 08:12 AM, Abhijeet Kasurde wrote:

+1

On 20/04/17 9:36 PM, Petr Vobornik wrote:

Hi all,

I'd like to improve quality of bug reports and RFEs.

A possibility I see is to create and issue template [1].

Sounds like a good idea! Please see my comments.


What do you think of the following template? Should we use it?

""""
### Request for enhancement
As  , I want <what?> so that <why?>.

This sounds very labored. How about using:
"I am a  and I want ..."


### Bug
 What doesn't work (what was the goal)

"What's not working" proposes the situation will change and
sounds better IMO




I took some inspiration from the Atom template. But tried to keep it 
shorter. As a bonus I added a link where people can find log files and a 
link to troubleshooting page.


New one:
"""
### Request for enhancement
As <persona, e.g. admin> , I want <what?> so that <why?>.

### Issue
[description of the issue]

 Steps to Reproduce
1.
2.
3.

 Actual behavior
(what happens)

 Expected behavior
(what do you expect to happen)

 Version/Release/Distribution
   $ rpm -q freeipa-server freeipa-client ipa-server ipa-client 
389-ds-base pki-ca krb5-server


 Additional info:
Any additional information, configuration, data or log snippets that is 
needed for reproduction or investigation of the issue.


Log file locations: 
https://access.redhat.com/documentation/en-US/Red_Hat_Enterprise_Linux/7/html/Linux_Domain_Identity_Authentication_and_Policy_Guide/config-files-logs.html

Troubleshooting guide: https://www.freeipa.org/page/Troubleshooting
"""






1.  Can we add pre-defined set of components in title ? for example,


I don't know if it is possible. Probably not.



[CERT] some_cert_related bug description
[installer] some installer related bug description

This is what Pagure has tags for. But you're right we might be missing
some, although "CERT" is probably not a good example, installer is. On
the other hand, "userstory" is a tag I will myself never use on purpose.


2. Also, Having a bot in place which will enforce or atleast suggest
reporter to modify bug report.


Could you elaborate?




[1] https://docs.pagure.org/pagure/usage/ticket_templates.html



My hope is that the issue template should do itself.

For the record, I love the way Atom guides you through their issue
creation: https://github.com/atom/atom/issues/new.



--
Petr Vobornik

Associate Manager, Engineering, Identity Management
Red Hat

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


[Freeipa-devel] Pagure issue template

2017-04-20 Thread Petr Vobornik

Hi all,

I'd like to improve quality of bug reports and RFEs.

A possibility I see is to create and issue template [1].

What do you think of the following template? Should we use it?

""""
### Request for enhancement
As  , I want <what?> so that <why?>.

### Bug
 What doesn't work (what was the goal)

 Steps to Reproduce

 Actual results

 Expected results

 Version/Release/Distribution
   $ rpm -q freeipa-server ipa-server 389-ds-base pki-ca krb5-server

 Additional info:

""""

[1] https://docs.pagure.org/pagure/usage/ticket_templates.html
--
Petr Vobornik

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [DRAFT] Release notes FreeIPA 4.5.0

2017-03-14 Thread Petr Vobornik

On 03/14/2017 04:24 PM, Martin Basti wrote:



On 14.03.2017 14:50, Jakub Hrozek wrote:

On Tue, Mar 14, 2017 at 01:51:19PM +0100, Martin Basti wrote:

Hello,

DRAFT for FreeIPA 4.5.0 release notes is ready
http://www.freeipa.org/page/Releases/4.5.0

Please update/let me know what is missing, what is extra.

Please update this paragraph:

AD User Short Names

Support for AD users short names has been added. Short
names can be enabled from CLI by setting ipa config-mod
--domain-resolution-order="domain.test:ad.domain1.test:ad.domain2.test"
or from WebUI under Configuration tab. No manual configuration on SSSD
side is required.


With a note that this feature is not supported by SSSD yet and the work
is tracked with https://pagure.io/SSSD/sssd/issue/3210


I updated that section. Shouldn't we remove it completely from release
notes because it will not work until new SSSD is released?



I'd keep it there and add Jakub's comment. It will be useful when SSSD 
with the support is released.



--
Petr Vobornik


--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] Samba 4.6.0-2.fc26 is available for trust tests

2017-03-10 Thread Petr Vobornik

On 03/10/2017 08:32 AM, Alexander Bokovoy wrote:

Hi,

I've submitted Samba 4.6.0-2 to FC26 and rawhide. This build contains
fixes that allow FreeIPA implement trust functionality under gssproxy
privilege separation. You need gssproxy 0.7.0 or later.

Please test and add karma to
https://bodhi.fedoraproject.org/updates/FEDORA-2017-c5e572f32b

There is no build for Fedora 25.



f25 build was added to @freeipa/freeipa-master COPR repo

--
Petr Vobornik

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] Migration of FreeIPA issue tracker - Trac and git repo to pagure.io

2017-03-01 Thread Petr Vobornik

On 02/28/2017 12:03 PM, Petr Vobornik wrote:

On 02/28/2017 12:00 PM, Petr Vobornik wrote:

On 02/27/2017 12:46 PM, Petr Vobornik wrote:

Hello list,

today and tomorrow a migration of FreeIPA issue tracker[1] and git repo
will take place.

It is due to FedoraHosted sunset [2]. Both will be migrated to pagure.io
[3].

During this migration it won't be possible to add new tickets and
comments to Trac or Pagure.

[1] https://fedorahosted.org/freeipa/
[2]
https://communityblog.fedoraproject.org/fedorahosted-sunset-2017-02-28/
[3] https://pagure.io/

Thank you for understanding,


Issue tracker and git repo were migrated. They can be used now.

https://pagure.io/freeipa

Additional steps will follow
- redirection of old URLs to new
- sync with github



Also we need to setup rights for the repo.

I've created group 'freeipa'. My proposal is to add all people who had
git commit rights to the group. Set the group to have 'commit' right on
'freeipa' pagure project.>



Former admins can be added as admins to the project directly.


I made everybody with former commit rights admins for now. The reason is 
that there is a bug that committer doesn't have right to change 
milestone and edit custom fields.


https://pagure.io/pagure/issue/2018
https://pagure.io/pagure/issue/2008



Martin2 is working on setting up sync with Git Hub:
- https://pagure.io/fedora-infrastructure/issue/5844




--
Petr Vobornik

Associate Manager, Engineering, Identity Management
Red Hat, Inc.

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] Migration of FreeIPA issue tracker - Trac and git repo to pagure.io

2017-02-28 Thread Petr Vobornik

On 02/28/2017 12:48 PM, Martin Basti wrote:



On 28.02.2017 12:38, Lukas Slebodnik wrote:

On (28/02/17 12:17), Martin Basti wrote:


On 28.02.2017 12:03, Petr Vobornik wrote:

On 02/28/2017 12:00 PM, Petr Vobornik wrote:

On 02/27/2017 12:46 PM, Petr Vobornik wrote:

Hello list,

today and tomorrow a migration of FreeIPA issue tracker[1] and git
repo
will take place.

It is due to FedoraHosted sunset [2]. Both will be migrated to
pagure.io
[3].

During this migration it won't be possible to add new tickets and
comments to Trac or Pagure.

[1] https://fedorahosted.org/freeipa/
[2]
https://communityblog.fedoraproject.org/fedorahosted-sunset-2017-02-28/

[3] https://pagure.io/

Thank you for understanding,

Issue tracker and git repo were migrated. They can be used now.

https://pagure.io/freeipa

Additional steps will follow
- redirection of old URLs to new
- sync with github


Also we need to setup rights for the repo.

I've created group 'freeipa'. My proposal is to add all people who had
git commit rights to the group. Set the group to have 'commit' right on
'freeipa' pagure project.

Former admins can be added as admins to the project directly.

Martin2 is working on setting up sync with Git Hub:
- https://pagure.io/fedora-infrastructure/issue/5844


and

https://pagure.io/fedora-infrastructure/issue/5845

Please do NOT push to old repository, for users of ipatool change your
repositories to pagure and would be good to postpone pushing until
mirroring
to github is enabled.


The best is to asg on fedora-infrastructure to chown the git repo on
fedorahosted, so no one can push changes there.

LS


Petr1 has a reason why it cannot be done, something with copr IIRC



It's something different. My solution was remove people from gitfreeipa 
group so they won't be able to push but that would also remove the 
rights to add packages to our COPR repository.


But IMO, until the sync with github is working, we should allow to push 
to fedora hosted, but only in a 'sync' way: pull from pagure, push to 
fedorahosted


--
Petr Vobornik

Associate Manager, Engineering, Identity Management
Red Hat, Inc.

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] Migration of FreeIPA issue tracker - Trac and git repo to pagure.io

2017-02-28 Thread Petr Vobornik

On 02/28/2017 12:00 PM, Petr Vobornik wrote:

On 02/27/2017 12:46 PM, Petr Vobornik wrote:

Hello list,

today and tomorrow a migration of FreeIPA issue tracker[1] and git repo
will take place.

It is due to FedoraHosted sunset [2]. Both will be migrated to pagure.io
[3].

During this migration it won't be possible to add new tickets and
comments to Trac or Pagure.

[1] https://fedorahosted.org/freeipa/
[2]
https://communityblog.fedoraproject.org/fedorahosted-sunset-2017-02-28/
[3] https://pagure.io/

Thank you for understanding,


Issue tracker and git repo were migrated. They can be used now.

https://pagure.io/freeipa

Additional steps will follow
- redirection of old URLs to new
- sync with github



Also we need to setup rights for the repo.

I've created group 'freeipa'. My proposal is to add all people who had 
git commit rights to the group. Set the group to have 'commit' right on 
'freeipa' pagure project.


Former admins can be added as admins to the project directly.

Martin2 is working on setting up sync with Git Hub:
- https://pagure.io/fedora-infrastructure/issue/5844

--
Petr Vobornik

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] Migration of FreeIPA issue tracker - Trac and git repo to pagure.io

2017-02-28 Thread Petr Vobornik

On 02/27/2017 12:46 PM, Petr Vobornik wrote:

Hello list,

today and tomorrow a migration of FreeIPA issue tracker[1] and git repo
will take place.

It is due to FedoraHosted sunset [2]. Both will be migrated to pagure.io
[3].

During this migration it won't be possible to add new tickets and
comments to Trac or Pagure.

[1] https://fedorahosted.org/freeipa/
[2] https://communityblog.fedoraproject.org/fedorahosted-sunset-2017-02-28/
[3] https://pagure.io/

Thank you for understanding,


Issue tracker and git repo were migrated. They can be used now.

https://pagure.io/freeipa

Additional steps will follow
- redirection of old URLs to new
- sync with github

--
Petr Vobornik

Associate Manager, Engineering, Identity Management
Red Hat, Inc.

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


[Freeipa-devel] Migration of FreeIPA issue tracker - Trac and git repo to pagure.io

2017-02-27 Thread Petr Vobornik

Hello list,

today and tomorrow a migration of FreeIPA issue tracker[1] and git repo 
will take place.


It is due to FedoraHosted sunset [2]. Both will be migrated to pagure.io 
[3].


During this migration it won't be possible to add new tickets and 
comments to Trac or Pagure.


[1] https://fedorahosted.org/freeipa/
[2] https://communityblog.fedoraproject.org/fedorahosted-sunset-2017-02-28/
[3] https://pagure.io/

Thank you for understanding,
--
Petr Vobornik

Associate Manager, Engineering, Identity Management
Red Hat, Inc.

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] Certificate Identity Mapping - new API to retrieve matching users

2017-02-22 Thread Petr Vobornik

On 02/22/2017 12:43 AM, Fraser Tweedale wrote:

On Tue, Feb 21, 2017 at 06:12:23PM +0100, Petr Vobornik wrote:

On 02/21/2017 05:15 PM, Florence Blanc-Renaud wrote:

Hi,

related to the Certificate Identity Mapping feature, a new CLI will be
needed to find all the users matching a given certificate.

I propose to provide this as:

ipa certmaptest --certificate 
---
2 users matched
---
  Matched user login: test1
  Matched user login: test2

Number of entries returned 2



Please provide any comments, suggestions on the CLI or the output.
Thanks,
Flo.



Thanks Flo for sharing it.

I don't like the command name. It is not self explanatory. It says it is
testing something, it is not clear what and the actual result is users who
match the map configuration or have the cert in their user's entry.

Better would be:
  $ ipa certmap-match --certificate


How about `ipa certmap-find-user ...'?  Doesn't get more obvious
than that, IMO.


Was thinking about that as well but I think that the command might, in 
future, return also something else then user object, e.g. ID override.






Pasting user story to give context if somebody is not familiar with it:
"""
As a Security Officer, I want to present IdM Server with an Employee Smart
Card certificate and list all Employees with a matching role account, so
that I can validate the configuration is correct

Note: In FreeIPA 4.4, user-find --certificate can already find users linked
with a certificate blob

Acceptance criteria:
* I can perform the administrative task both via IdM Web UI and CLI
* When asking IdM for the information, I should always receive the same list
that would be matched in client authentication workflows (by SSSD)
* The list of users should include both users linked via standard
certificate blob and other generically mapped users
"""
--
Petr Vobornik

Associate Manager, Engineering, Identity Management
Red Hat, Inc.

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code



--
Petr Vobornik

Associate Manager, Engineering, Identity Management
Red Hat, Inc.

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] Certificate Identity Mapping - new API to retrieve matching users

2017-02-21 Thread Petr Vobornik

On 02/21/2017 05:15 PM, Florence Blanc-Renaud wrote:

Hi,

related to the Certificate Identity Mapping feature, a new CLI will be
needed to find all the users matching a given certificate.

I propose to provide this as:

ipa certmaptest --certificate 
---
2 users matched
---
  Matched user login: test1
  Matched user login: test2

Number of entries returned 2



Please provide any comments, suggestions on the CLI or the output.
Thanks,
Flo.



Thanks Flo for sharing it.

I don't like the command name. It is not self explanatory. It says it is 
testing something, it is not clear what and the actual result is users 
who match the map configuration or have the cert in their user's entry.


Better would be:
  $ ipa certmap-match --certificate


Pasting user story to give context if somebody is not familiar with it:
"""
As a Security Officer, I want to present IdM Server with an Employee 
Smart Card certificate and list all Employees with a matching role 
account, so that I can validate the configuration is correct


Note: In FreeIPA 4.4, user-find --certificate can already find users 
linked with a certificate blob


Acceptance criteria:
* I can perform the administrative task both via IdM Web UI and CLI
* When asking IdM for the information, I should always receive the same 
list that would be matched in client authentication workflows (by SSSD)
* The list of users should include both users linked via standard 
certificate blob and other generically mapped users

"""
--
Petr Vobornik

Associate Manager, Engineering, Identity Management
Red Hat, Inc.

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [design] add nsupdate output format to dns-update-system-records

2017-01-27 Thread Petr Vobornik
On 01/27/2017 01:37 PM, Martin Basti wrote:
> Hello all,
> 
> I'm working on adding support of nsupdate format to output of `ipa
> dns-update-system-records` command, that will allow to copy output
> to nsupdate utility and update IPA DNS records on external server. I
> propose following:
> 
> 1) new option --nsupdate-format
> 
> This option will replace standard output with nsupdate format output.
> Feel free to propose better name.
> 
> 
> 2) client or server side plugin?
> 
> I'd like to implement this behavior only at client side. But this will
> not work in cases when only server api or RPC calls are used. Do you
> think that we should support this uses case (in server side plugin)?
> 
> 
> Martin
> 


The main ideal was to allow piping into nsupdate. i don't remember if
server side returns in human readable form or in structured way. If in
structured, then implementation in client side plugin would make better
sense.

+1 to Alexander on '--out nsupdate' option

-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [DESIGN] FreeIPA on FIPS + NSS question

2017-01-13 Thread Petr Vobornik
>> reason).
>>>>>>>
>>>>>>> Will be happy to hear thoughts on this,
>>>>>>> Standa
>>>>>> I'm not a big fan of NSS, it has it's issues. As the author of the
>>>>>> Python binding I'm quite aware of all the nasty behaviors NSS has and
>>>>>> needs to be worked around. I wouldn't be sad to see it go but OpenSSL
>>>>>> has it's own issues too. If you remove NSS you're also removing the
>>>>>> option to support smart cards, HSM's etc. Perhaps before removing
>>>>>> functionality it would be good to assess what the requirements are.
>>>>>>
>>>>> I'm sorry I generalized too much, the original topic was moving away
>>>>> from python-nss (of which I am even more sorry as you're the author).
>>>>>
>>>> We could use some ideas on how to handle replica installations in FIPS.
>>>>
>>>> We might use some flag in LDAP to indicate that a topology is
>>>> FIPS-enabled. It seems like a good idea to force all servers in
>>>> FIPS-enabled topology to also be FIPS-enabled. At the start of replica
>>>> installation, a check could be performed to verify the FIPS topology
>>>> status is the same as the current system's FIPS status. However, this
>>>> proposal has a flaw. It is possible to simply install a FIPS-enabled
>>>> replica and then turn FIPS off. This would result in non-FIPS systems
>>>> being part of a FIPS-enabled topology.
>>>>
>>>> So we have a couple questions:
>>>>
>>>> Does it make sense to require all the servers in the topology to be
>>>> either FIPS-enabled or FIPS-disabled?
>>>> What would be a good approach to achieve this? Simply checking during
>>>> installation does not guarantee that FIPS will stay turned on.
>>>>
>>> You could set some value in the replicated tree on FIPS status and write
>>> a 389-ds plugin to refuse to start if the environment doesn't match.
>>> Given this is started first it should cause a cascade of failures so no
>>> services are available.
>>>
>>> rob
>> Based on an offline discussion, we might just omit this feature
>> entirely. Our goal is to enable FreeIPA installation of FIPS configured
>> systems. It should be up to the customer to make sure other FIPS
>> requirements are met, i.e. all servers are configured to use FIPS.
>>
> 
> Up to you but this is bound to be an RFE and seems like quite a weakness
> to me.
> 
> You'll want to be sure to enforce that any additional masters get FIPS
> by default if the initial master has it. In other words, they shouldn't
> need to remember to pass an option, it should be automatic.
> 
> rob
> 

+1 to Rob.

We could operate between the two extremes:
  * Extreme 1: Ensuring that FIPS will remain enabled on all existing
replica.
  * Extreme 2: Do not validate anything

I.e. we could only:
  * Note in LDAP that first server was installed with FIPS
  * Check it at install time
- prevent from installation of non-FIPS
- prevent from installation of FIPS replica in non-FIPS topology

It won't be bullet proof but it will check major mistakes which the
admin can do by accident.

-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] FreeIPA, Duo Security integration

2017-01-05 Thread Petr Vobornik
On 01/05/2017 09:36 AM, Oucema Bellagha wrote:
> Hi,
> As of now, we have FreeIPA with OTP working perfectly.  Now, I am looking at 
> possibly integrating Duo security instead of FreeIPA's 2FA.  I am concerned 
> about how it will fit in with FreeIPA... Has anyone else tried this before?  
> If 
> so, are there any pitfalls or problems you have encountered or any general 
> advise?
> 
> Cheers, Euqanra'l --
> 

Hello Oucema,

Integration with other 2FA system can be handled by RADIUS proxy
feature:
https://access.redhat.com/documentation/en-US/Red_Hat_Enterprise_Linux/7/html/Linux_Domain_Identity_Authentication_and_Policy_Guide/otp.html#migrating-proprietary-otp

For practical experience with Duo, better ask on freeipa-users mailing
list where admin community dwells. freeipa-devel is primarily used for
development discussions.

Btw, what is the use case or reasons to integrate with Duo instead of
using FreeIPA's 2FA?
-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] client-only FreeIPA build

2016-11-22 Thread Petr Vobornik
On 11/22/2016 05:25 PM, Rob Crittenden wrote:
> Lukas Slebodnik wrote:
>> On (22/11/16 16:29), Petr Spacek wrote:
>>> On 22.11.2016 16:27, Jan Cholasta wrote:
>>>> Hi,
>>>>
>>>> On 22.11.2016 16:04, Petr Spacek wrote:
>>>>> Hello,
>>>>>
>>>>> the recent changes with regard to
>>>>> http://www.freeipa.org/page/V4/Integration_Improvements
>>>>> beg a question whether we should invest into supporting client-only 
>>>>> builds in
>>>>> FreeIPA build system.
> 
> Note that the Integration efforts don't really apply. The client-only
> install is for doing client enrollment and integration can mean lots of
> things.
> 
>>>>>
>>>>> Right now, FreeIPA can be built on all architectures we care about so 
>>>>> there is
>>>>> no incentive to invest into client-only build - this applies to binary/RPM
>>>>> builds.
>>>>
>>>> Client-only build lowers the barrier for porting IPA to new platforms 
>>>> (porting
>>>> only client code is *much* easier than porting the whole thing), so I would
>>>> very much prefer if we kept it.
>>>
>>> Understood.
>>>
>> Agree about portability
>>
>> But upstream spec file needn't have such relicts.
>> The upstream spec file is pure fedora specific.
> 
> The upstream spec is what is used to document and verify that the
> client-only build actually works.
> 
> I also think it is a worthy goal to maintain.
> 
>>> Wondering out loud: What prevents the "porter" from doing full build and 
>>> then
>>> packaging only client bits? Yes, he has to install come of the dependencies
>>> for the build to pass but still, it is way easier than actually making 
>>> server
>>> fully functional.
> 
> It is not an insignificant amount of dependencies to build all of IPA.
> 
>>> Petr, are you going to allocate time for this soonish or should I open a
>>> ticket and forget about it for now?
> 
> IMHO this should be covered under the build refactoring to avoid
> regressions.
> 
> rob
> 

I think we should not implement it. I see no need. Fedora, Debian, RHEL
all work with server build. Difference between arches is not issue as well.

If somebody would want it for porting IPA on other distro then fine. But
at that stage there will be more stuff to figure out so writing the
patch can wait for that eventuality.

-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH] webui: 0084, 0101: refactoring rpc module

2016-11-10 Thread Petr Vobornik
On 08/09/2016 01:29 PM, Pavel Vomacka wrote:
> Hello,
> 
> please review attached patches.
> 
> The rpc module is now separated from display layer
> and changing activity text while loading metadata.
> 
> https://fedorahosted.org/freeipa/ticket/6144
> 
> 
> 

patch 84:

Looks good, works fine, it just needed rebase(I could provide that).

Idea, but that doesn't have to be implemented, or sometime in future,
right now it is not useful: What about providing the rpc object in the
event, and having unique id for each rpc call so that we could track all
rpc which are executed.


patch 101:

1. It's event name but the property name looks like that it contains a text:
   that.change_text = 'change-activity-text';

Should it be rather: that.change_text_event.

Or even, why does it compare previous text? Does it matter? Wouldn't be
better to have 'set-activity' event. And then the handler would call
something new set_text method:

set_text(new_activity)
  that.dots = 0
  that.text = new_activity
  that.make_step()


-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH] webui: Fix coverity bugs

2016-11-10 Thread Petr Vobornik

Commenting only on top, it's too long.

ACK for everything. I've rebased patch 90.

pushed to master

master:
* a2525ff64518038eaa64b0d855154a984030f7f3 Coverity - null pointer exception
* d4ad0ca04c0ae445c784787a675ac84d2cbfd766 Coverity - null pointer exception
* fa3982c7c82add3d201aec860cb981a595f10be9 Coverity - not initialized
variable
* de8cb7585b652fd1a61e3020e37192cb1db74f46 Coverity - identical code for
different branches
* 4b63ce26ebbef8ef1538aecb3cff8032df3357a7 Coverity - Accesing attribute
of null
* ed74e14ab4a17c83cf6782e4b6fd41a2ce79594d Coverity - removed dead code
* 7be585dbb206ed12b25d09bfb2f5452ee9c125ae Coverity - true branch can't
be executed
* d94a2aa185defba38f2bbe2c5ee28f9b9defc0f2 Coverity - true branch can't
be executed
* cad9f9b682d9bcc33fdfb1112e4cfb1a2c66a498 Coverity - null pointer
dereference
* 4af31c70c57fc223920b71fedfb40d1de27622b2 Coverity - iterating over
variable which could be null
* cd74f78ed74f8898c492024d0901cef9778df067 Coverity - opens dialog which
might not be created
* aa8a904c4a3953e799278de192d1613d21cde42a Coverity - accessing
attribute of variable which can point to null
* 2644c955489ee5b22ecc0227c5cd8ed1e90ee648 Coverity - null pointer
dereference


On 08/05/2016 02:33 PM, Pavel Vomacka wrote:
> 
> 
> On 08/01/2016 05:53 PM, Petr Vobornik wrote:
>> On 07/29/2016 03:25 PM, Alexander Bokovoy wrote:
>>> On Fri, 29 Jul 2016, Pavel Vomacka wrote:
>>>> Hello,
>>>>
>>>> please review attached patches which fixes errors from Coverity.
>>>>
>>>> -- 
>>>> Pavel^3 Vomacka
>>>>
>>>>  From 0391289b3f6844897e2a9f3ae549bd4c33233ffc Mon Sep 17 00:00:00 2001
>>>> From: Pavel Vomacka <pvoma...@redhat.com>
>>>> Date: Mon, 25 Jul 2016 10:36:47 +0200
>>>> Subject: [PATCH 01/13] Coverity - null pointer exception
>>>>
>>>> Variable 'option' can be null and there will be error of reading
>>>> property of null.
>>>> ---
>>>> install/ui/src/freeipa/widget.js | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/install/ui/src/freeipa/widget.js
>>>> b/install/ui/src/freeipa/widget.js
>>>> index
>>>> 9151ebac9438e9e674f81bfb1ccfe7a63872b1ae..cfdf5d4750951e4549c16a2b9b9c355f61e90c39
>>>>
>>>> 100644
>>>> --- a/install/ui/src/freeipa/widget.js
>>>> +++ b/install/ui/src/freeipa/widget.js
>>>> @@ -2249,7 +2249,7 @@ IPA.option_widget_base = function(spec, that) {
>>>>  var child_values = [];
>>>>  var option = that.get_option(value);
>>>>
>>>> -if (option.widget) {
>>>> +if (option && option.widget) {
>>>>  child_values = option.widget.save();
>>>>  values.push.apply(values, child_values);
>>>>  }
>>>> -- 
>>>> 2.5.5
>>>>
>>> ACK
>> ACK
>>
>>>>  From 6df8e608232e25daa9aefe4fccbdeca4dbaf1998 Mon Sep 17 00:00:00 2001
>>>> From: Pavel Vomacka <pvoma...@redhat.com>
>>>> Date: Mon, 25 Jul 2016 10:43:00 +0200
>>>> Subject: [PATCH 02/13] Coverity - null pointer exception
>>>>
>>>> Variable 'row' could be null in some cases. And set css to variable
>>>> which is pointing to null
>>>> causes error. Therefore there is new check.
>>>> ---
>>>> install/ui/src/freeipa/widget.js | 2 ++
>>>> 1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/install/ui/src/freeipa/widget.js
>>>> b/install/ui/src/freeipa/widget.js
>>>> index
>>>> cfdf5d4750951e4549c16a2b9b9c355f61e90c39..5844436abf090f12d5a9d65efe7a1aaee14097e2
>>>>
>>>> 100644
>>>> --- a/install/ui/src/freeipa/widget.js
>>>> +++ b/install/ui/src/freeipa/widget.js
>>>> @@ -5766,6 +5766,8 @@ exp.fluid_layout = IPA.fluid_layout =
>>>> function(spec) {
>>>>  that.on_visible_change = function(event) {
>>>>
>>>>  var row = that._get_row(event);
>>>> +if (!row) return;
>>>> +
>>>>  if (event.visible) {
>>>>  row.css('display', '');
>>>>  } else {
>>>> -- 
>>>> 2.5.5
>>>>
>>> ACK
>>
>> ACK
>>
>>>
>>>>  From 6f2ddc9e1c5323a640bdf744d2da00bfee7ab766 Mon Sep 17 00:00:00 2001
>>>> From: Pavel 

Re: [Freeipa-devel] Script to setup Kerberized NFS exports using IPA

2016-11-07 Thread Petr Vobornik
On 11/07/2016 05:49 PM, Martin Babinsky wrote:
> On 11/07/2016 05:43 PM, Justin Mitchell wrote:
>> I have been working on a python script to setup secure NFS exports using
>> kerberos that relies heavily on FreeIPA, and is in many ways the server
>> side compliment to ipa-client-automount. It attempts to automatically
>> discover the setup, and falls back to asking simple questions, in the
>> same way as ipa-server-install et al do.
>>
>> I'm not sure quite where it would fit best in the freeipa source tree,
>> perhaps under 'client' ?
>> Also, whats would be the best way to submit the script, as a patch or a
>> github pull request ?
>>
>> thanks
>>
>>
>>
> 
> If it is a server-side code then it should go into ipaserver/ namespace.

Could you describe the use case in more details?

IIUIC it's about configuring NFS server against IPA and not IPA server
itself as NFS server. In that case it should be IMO in client package
because NFS server is also a client from IPA's perspective.

> 
> We now prefer contributions in form of Github pull-requests.

Right
-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH] 956 replicainstall: log ACI and LDAP errors in promotion check

2016-10-26 Thread Petr Vobornik
On 10/26/2016 09:53 AM, Martin Basti wrote:
> 
> 
> On 30.03.2016 10:06, Martin Basti wrote:
>>
>>
>> On 24.03.2016 15:27, Petr Vobornik wrote:
>>> to enable debugging of such errors.
>>>
>>> E.g.: https://fedorahosted.org/freeipa/ticket/5741
>>>
>>>
>> Can we log the whole traceback to get exact place where error happened?
>>
>> Martin^2
>>
>>
> bump
> 

replaced by: https://github.com/freeipa/freeipa/pull/186


-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


[Freeipa-devel] Limiting pull request notification sizes

2016-10-19 Thread Petr Vobornik
Looking at: [Freeipa-devel] [freeipa PR#171][synchronized] Build system
cleanup phase 2

I see that the attached freeipa-pr-171.patch has 7.8 MB. With couple
forced pushed to the private branch it creates quite big traffic a takes
space.

Is it possible to limit sizes of these attachments to let's say 50KB?

I.e., I would be interested in the small patches but let's read the
large ones on GitHub.
-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [help]

2016-10-19 Thread Petr Vobornik
On 10/19/2016 10:03 AM, Martin Basti wrote:
> 
> 
> On 19.10.2016 03:35, 郑磊 wrote:
>> Hello,
>> In the process of using freeipa, we found a lot of content without Chinese 
>> translation. In order to we can better use the platform,
> 
>> we made a Chinese translation.
> Sorry but I don't see any updates in zanata 
> https://fedora.zanata.org/iteration/view/freeipa/master/languages/zh-CN?dswid=-4750
> 
> If you do a custom/manual translations this will not scale and you will do 
> the 
> same every release
> 
>> I think the translation work for freeipa internationalization promotion also 
>> can have certain help. In addition, in use process, we found that when test 
>> the corresponding function of operation in the Web interface is not a 
>> straightforward logging which needs to query in the background, and it may 
>> not 
>> be intuitive and convenient. In order to we can intuitively record what we 
>> have done the operation in the Web interface, we do the log module plug-ins.
>>
>>
> webUI is just layer on top of our API calls.

Note that there are some parts of Web UI that aren't translated. Mainly
login page. It's because translations are fetched by API call which
needs successful auth.
> 
> 
>>
>>
>>
>> --
>> 祝:
>> 工作顺利!生活愉快!
>> --
>> 长沙研发中心 郑磊
>> Phone:18684703229
>> Email:zheng...@kylinos.cn
>> Company:天津麒麟信息技术有限公司
>> Address:湖南长沙市开福区三一大道工美大厦十四楼
>> -- Original --
>> *From: * "Martin Basti"<mba...@redhat.com>;
>> *Date: * Tue, Oct 18, 2016 06:50 PM
>> *To: * "郑磊"<zheng...@kylinos.cn>; "freeipa-devel"<freeipa-devel@redhat.com>;
>> *Subject: * Re: [Freeipa-devel] [help]
>>
>>
>>
>> On 18.10.2016 03:28, 郑磊 wrote:
>>> Hello everyone,
>>> I'm using freeipa, and having a test and research with the function of 
>>> freeipa. At the same time, I have carried on the Chinese translation to the 
>>> web interface, also added own log module in web interface, referring to the 
>>> following screenshots. However, for these changes I don't know how to 
>>> interact with the community. Is there a administrator to review these 
>>> changes? Who should I send mail to? Please help me. Thank you very much!
>>>
>>>
>>>
>>
>> Hello,
>>
>> at first you can write here what is your goal, what are your use cases and 
>> how 
>> do you plan to achieve that.
>>
>> Then, if we agree on solution, you can send code as pull request to our 
>> github 
>> repository https://github.com/freeipa/freeipa
>>
>>
>> Martin^2
> 
> 
> 


-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] Broken IPA installation caused by new python-dns package

2016-10-13 Thread Petr Vobornik
On 10/12/2016 11:11 AM, Petr Spacek wrote:
> On 10.10.2016 10:28, Martin Basti wrote:
>> https://bodhi.fedoraproject.org/updates/FEDORA-2016-1857421df6
>>
>>
>> Please set karma accordingly
>>
>>
>> Traceback:
>>
>> ...
>>
>> 2016-10-10T04:44:05Z DEBUG The ipa-replica-install command failed, exception:
>> TypeError: 'unicode' does not have the buffer interface
>> 2016-10-10T04:44:05Z ERROR 'unicode' does not have the buffer interface
>>
>>
>> I'll investigate if IPA using it wrong or there is new error introduced in
>> pyhton-dns
> 
> For archaeologists:
> Fix
> https://github.com/freeipa/freeipa/pull/150
> was merged.
> 

We've pushed PR 150 to 4.4 and master. 4.4.2 release fixes f25 and f26
but F24 has 4.3 branch.

Is it correct to assume that 4.3 is also affected?

If so, then we need either to backport the patch to 4.3 and fix Fedora
directly or completely block the python-dns update on f24.
-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] Feature branches for sub-team efforts

2016-10-11 Thread Petr Vobornik
On 10/11/2016 03:50 PM, Alexander Bokovoy wrote:
> On ti, 11 loka 2016, Petr Vobornik wrote:
>> Hi List,
>>
>> we discussed locally a proposal about creating a feature branch for each
>> sub-team effort in our main git. Currently it would be for the 4 ongoing
>> refactoring efforts + Simo's work
>>
>> Why?
>> It will allow each developer to create a pull request against the
>> feature branch and thus it will enable iterative development by multiple
>> devs without affecting other sub-teams. When the feature(refactoring) is
>> finished, the branch would be rebased on master and merged there. Note:
>> rebases can be done as needed - e.g. when other subteam finishes its
>> work.
>>
>> Concerns:
>> 1. Upstream git repo would be full of such branches.
>> - This can be mitigated by deleting the feature branches when their are
>> released or merged(up to discussion)
> Don't put them in the upstream git repo. Let people decide where they
> want to have them -- all Fedora contributors have access to
> fedorapeople.org git hosting and there is github one button click
> ('Clone') away from the github mirror.
> 
> It is not a problem to keep a separate git branch published this way.
> 

It is not a matter of making the code public. That can be done easily as
you write. Other alternative is own branch in GitHub fork.

> May be I misunderstand you -- if you just want people to propose merge
> requests on github with pre-defined names, then that's just fine.
> 

Basically it's all about review.

Example use case: More than 1 devs are working on a same big effort.
This effort will probably consists of 10s of commits. The big effort is
divided into smaller ones which can be implemented and reviewed
separately. In our previous mode, the sub task would be merged to master
it is reviewed and ACKed. But now we cannot do that. We want to merge
the whole big task at once when it is finishes and tested.

One dev could probably have a branch on personal fork of FreeIPA on
GitHub which would work as the feature branch. Other team members would
create pull requests against it.

In such case we would loose mail notifications and would have to extend
our tooling because ipatool can use only one upstream on push.

Pre-defined names for PRs is a good idea - we could easily see what
effort the pull request is related to.
-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] Build system refactoring - design document

2016-10-11 Thread Petr Vobornik
On 10/07/2016 11:56 AM, Petr Spacek wrote:
> Dear FreeIPA developers and packagers,
> 
> you can find first version of the Build system refactoring design document on:
> http://www.freeipa.org/page/V4/Build_system_refactoring
> 
> If you do not care about implementation details, please be so kind and quickly
> scan through chapter
> http://www.freeipa.org/page/V4/Build_system_refactoring#Feature_Management
> 
> I'm not an FreeIPA packager so I might miss some important thing which needs
> to be configurable.
> 
> 
> Also, I would appreciate ideas how to handle build versioning:
> http://www.freeipa.org/page/V4/Build_system_refactoring#Versioning
> 
> My main questions are:
> * What is triggering IPA upgrade?
> * Would it be sufficient to bump release in RPM? (I mean - theoretically.
> Could the code be modified to detect this?)
> 
> Here I'm trying to avoid unnecessary rebuilds caused by changes to
> IPA_VENDOR_VERSION during each build.
> 
> 
> Timo, what can I do to help you with packaging for Ubuntu/Debian?
> 
> Dream big, even wild ideas are welcome!
> 

I'd like to add one use case which is a prerequisite for "packager":
release engineer.

Currently, IPA is released by running
  $ make IPA_VERSION_IS_GIT_SNAPSHOT=no rpms

Then tarball is copied from dist/sources to freeipa.org

http://www.freeipa.org/page/Release#Building_the_sources

With current code, it may be done only with:
 $ make tarball

But it probably wasn't tested much so I'd not rely on it.

What I'd like to see:

Release engineer:
 $ make dist
 $ # copy tarball

Packager:
 $ ./configure [--options]
 $ make install

I think that this workflow is implied by "Automake: Standard Targets"
but IMHO it should be specified in the design explicitly because it is a
change in our process.

-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


[Freeipa-devel] Feature branches for sub-team efforts

2016-10-11 Thread Petr Vobornik
Hi List,

we discussed locally a proposal about creating a feature branch for each
sub-team effort in our main git. Currently it would be for the 4 ongoing
refactoring efforts + Simo's work

Why?
It will allow each developer to create a pull request against the
feature branch and thus it will enable iterative development by multiple
devs without affecting other sub-teams. When the feature(refactoring) is
finished, the branch would be rebased on master and merged there. Note:
rebases can be done as needed - e.g. when other subteam finishes its work.

Concerns:
1. Upstream git repo would be full of such branches.
- This can be mitigated by deleting the feature branches when their are
released or merged(up to discussion)

2. Too big traffic on a list.
- IMO we actually want it - progress will be visible to others


Naming:
I propose:
  refactoring-XXX
  feature-XXX

Thoughts? Anyone against?
-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] 4.4.2 release notes draft

2016-10-06 Thread Petr Vobornik
On 10/05/2016 06:40 PM, Petr Vobornik wrote:
> Hi,
> 
> we planned to release 4.4.2 Today. I'd postpone it to tomorrow morning
> so you have time to read the RN page.
> 
> Almost completely auto-generated release notes page:
> http://www.freeipa.org/page/Releases/4.4.2
> 
> Please help to to highlight important bug fixes.
> 
> Notes:
> - the only manual part is "Known Issues section"
> - the script for generating RN will be shared
> 

The release will be postponed because of regression introduced in 4.4.2:
- https://fedorahosted.org/freeipa/ticket/6385

-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


[Freeipa-devel] 4.4.2 release notes draft

2016-10-05 Thread Petr Vobornik
Hi,

we planned to release 4.4.2 Today. I'd postpone it to tomorrow morning
so you have time to read the RN page.

Almost completely auto-generated release notes page:
http://www.freeipa.org/page/Releases/4.4.2

Please help to to highlight important bug fixes.

Notes:
- the only manual part is "Known Issues section"
- the script for generating RN will be shared
-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH 0097] Properly handle LDAP socket closures in ipa-otpd

2016-09-30 Thread Petr Vobornik
On 09/28/2016 04:58 PM, Nathaniel McCallum wrote:
> On Wed, 2016-09-28 at 08:03 +0300, Alexander Bokovoy wrote:
>> On ti, 27 syys 2016, Nathaniel McCallum wrote:
>>> In at least one case, when an LDAP socket closes, a read event is
>>> fired
>>> rather than an error event. Without this patch, ipa-otpd silently
>>> ignores this event and enters a state where all bind auths fail.
>>>
>>> To remedy this problem, we pass error events along the same path as
>>> read events. Should the actual read fail, we exit.
>>
>> Please add the bugzilla link.
> 
> Done.
> 

Linked upstream ticket: https://fedorahosted.org/freeipa/ticket/6368
-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] FedoraHosted.org sunset

2016-09-23 Thread Petr Vobornik
On 09/23/2016 02:09 PM, Martin Basti wrote:
> 
> 
> On 23.09.2016 09:54, Jakub Hrozek wrote:
>> On Thu, Sep 22, 2016 at 06:09:43PM +0200, Petr Vobornik wrote:
>>> Hi all,
>>>
>>> As you know, FedoraHosted.org will be decommissioned.
>>>  
>>> https://communityblog.fedoraproject.org/fedorahosted-sunset-2017-02-28/
>>>
>>> We use Trac instance there. Let's discuss where we should migrate and
>>> what are our requirements. Then put results on one place. For that I've
>>> created:
>>>http://www.freeipa.org/page/FedoraHosted_Migration
>> That you for writing this up, there are some good points I didn't think
>> about, like migrating the ticket numbers. Did you already file an issue
>> that tracks this in Pagure (or asked if this is already possible)?
>>
> 
> Do we need review by field? It is recorded in commit and for ongoing
> reviews we are assigning ourselves to pull requests, so everybody knows
> if somebody is reviewing a PR.
> 
> Martin^2
> 

Assigning to PR solves the issue for which "review by" was meant. So we
may eventually drop it. Ideally when patch backlog on devel list is
cleansed.

In general, I'd not say that each individual field is a requirement,
e.g. I can imagine that: keywords, source, component, maybe even a
milestone and a priority can be tags/labels

What would be nice is some reporting/filtering capability so that I
don't have to script each view separately.
-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


[Freeipa-devel] FedoraHosted.org sunset

2016-09-22 Thread Petr Vobornik
Hi all,

As you know, FedoraHosted.org will be decommissioned.
 https://communityblog.fedoraproject.org/fedorahosted-sunset-2017-02-28/

We use Trac instance there. Let's discuss where we should migrate and
what are our requirements. Then put results on one place. For that I've
created:
  http://www.freeipa.org/page/FedoraHosted_Migration

It already contains several requirements which were discussed in other
channels.
-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [DESIGN][UPDATE] Time-Based HBAC Policies

2016-08-26 Thread Petr Vobornik
On 08/26/2016 12:47 PM, Standa Laznicka wrote:
> On 08/26/2016 12:39 PM, Martin Basti wrote:
>>
>>
>> On 26.08.2016 12:37, Petr Vobornik wrote:
>>> On 08/26/2016 12:23 PM, Martin Basti wrote:
>>>>
>>>> On 26.08.2016 12:20, Alexander Bokovoy wrote:
>>>>> On Fri, 26 Aug 2016, Jan Cholasta wrote:
>>>>>> On 26.8.2016 11:55, Martin Basti wrote:
>>>>>>>
>>>>>>> On 26.08.2016 11:43, Jan Cholasta wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> On 11.8.2016 12:34, Stanislav Laznicka wrote:
>>>>>>>>> Hello,
>>>>>>>>>
>>>>>>>>> I updated the design of the Time-Based HBAC Policies according
>>>>>>>>> to the
>>>>>>>>> discussion we led here earlier. Please check the design page
>>>>>>>>> http://www.freeipa.org/page/V4/Time-Based_Account_Policies. The
>>>>>>>>> biggest
>>>>>>>>> changes are in the Implementation and Feature Management
>>>>>>>>> sections. I
>>>>>>>>> also added a short How to Use section.
>>>>>>>> 1) Please use the 'ipa' prefix for new attributes:
>>>>>>>> memberTimeRule ->
>>>>>>>> ipaMemberTimeRule
>>>>>>>>
>>>>>>>>
>>>>>>>> 2) Source hosts are deprecated and thus should be removed from
>>>>>>>> ipaHBACRuleV2.
>>>>>>>>
>>>>>>>>
>>>>>>>> 3) Since time rules are defined by memberTimeRule, accessTime
>>>>>>>> should
>>>>>>>> be removed from ipaHBACRuleV2.
>>>>>>> ad 2) 3)
>>>>>>>
>>>>>>> Because backward compatibility, ipaHBACRuleV2 must contain all
>>>>>>> attributes from ipaHBACRule as MAY
>>>>>> Not true.
>>>>>>
>>>>>>> With current approach, when timerule is added to HBAC, we just
>>>>>>> change
>>>>>>> objectclass from 'ipahbacrule' to 'ipahbacrulev2' so we keep all
>>>>>>> attributes that was defined in older HBAC. Removing any attrs from
>>>>>>> ipaHBACRuleV2 can cause schema violation.
>>>>>> Which is perfectly fine.
>>>>>>
>>>>>>>
>>>>>>> I'm not sure if want to handle this in code (removing deprecated
>>>>>>> attributes from HBAC entry when timerule is added)
>>>>>> We don't have to do anything. If any of the deprecated attributes are
>>>>>> present when you change the object class (which they shouldn't
>>>>>> anyway), you'll get schema violation, otherwise it will work just
>>>>>> fine.
>>>>>>
>>>>>>> I realized that AccessTime is MUST for 'ipahbacrule', so when
>>>>>>> timerule
>>>>>>> ('ipahbacrulev2') is removed and somebody deleted accesstime we
>>>>>>> have to
>>>>>>> add it back.
>>>>>> It is MAY. The only MUST attribute is accessRuleType, but that is
>>>>>> deprecated as well and should be removed from ipaHBACRuleV2. We only
>>>>>> support allow rules, so when timerule is removed, that's the value
>>>>>> you set accessRuleType to.
>>>>> SSSD does search for HBAC rules by '(objectclass=ipaHBACRule)' filter.
>>>>> Changing to ipaHBACRuleV2 means these rules will not be found by older
>>>>> SSSD versions and therefore people will experience problems with older
>>>>> clients not being able to use new rules even if they would lack time
>>>>> component.
>>>>>
>>>> Older client do not support timerules, so they should not search for
>>>> them. HBAC without timerules will be still have 'ipaHBACRule'
>>>> objectclass and will work with old clients. Only HBAC with timerules
>>>> will have assigned 'ipaHBACRuleV2'
>>>>
>>>> Martin^2
>>> I miss "why" part of "To be able to handle backward compatibility with
>>> ease, a new object called ipaHBACRulev2 is introduced. " in the design
>>> page. If the reason is the above - old client's should ignore time rules
>>> then it has to be mentioned there. Otherwise I don't see a reason to
>>> introduce a new object type instead of extending the current.
>>
> It's exactly that - I will mention it there, then.

Thanks

>> How do you want to enforce HBAC rule that have set time from 10 to 14
>> everyday? With the same objectclass old clients will allow this HBAC
>> for all day. Isn't this CVE?
>>
> Word.
>>>
>>>
>>> 2. About API and CLI: wasn't there an idea to hide/not provide
>>> --icalfile=file.ics and --time=escaped_icalstring  options in the first
>>> implementation. So that we can limit the support scope to only a subset
>>> of option(the  OPTS part). If arbitrary ical is allowed since the
>>> beginning then we are asking for a lot of bugs filed.
>>>
>>
> Why hide it if there's no real problem with it? The string/content only
> has to be cut down to the restrictions of one event per VCALENDAR but I
> do not see the problem there.
> 

OK then.
-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [DESIGN][UPDATE] Time-Based HBAC Policies

2016-08-26 Thread Petr Vobornik
On 08/26/2016 12:39 PM, Martin Basti wrote:
> 
> 
> On 26.08.2016 12:37, Petr Vobornik wrote:
>> On 08/26/2016 12:23 PM, Martin Basti wrote:
>>>
>>> On 26.08.2016 12:20, Alexander Bokovoy wrote:
>>>> On Fri, 26 Aug 2016, Jan Cholasta wrote:
>>>>> On 26.8.2016 11:55, Martin Basti wrote:
>>>>>>
>>>>>> On 26.08.2016 11:43, Jan Cholasta wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> On 11.8.2016 12:34, Stanislav Laznicka wrote:
>>>>>>>> Hello,
>>>>>>>>
>>>>>>>> I updated the design of the Time-Based HBAC Policies according
>>>>>>>> to the
>>>>>>>> discussion we led here earlier. Please check the design page
>>>>>>>> http://www.freeipa.org/page/V4/Time-Based_Account_Policies. The
>>>>>>>> biggest
>>>>>>>> changes are in the Implementation and Feature Management
>>>>>>>> sections. I
>>>>>>>> also added a short How to Use section.
>>>>>>> 1) Please use the 'ipa' prefix for new attributes: memberTimeRule ->
>>>>>>> ipaMemberTimeRule
>>>>>>>
>>>>>>>
>>>>>>> 2) Source hosts are deprecated and thus should be removed from
>>>>>>> ipaHBACRuleV2.
>>>>>>>
>>>>>>>
>>>>>>> 3) Since time rules are defined by memberTimeRule, accessTime should
>>>>>>> be removed from ipaHBACRuleV2.
>>>>>> ad 2) 3)
>>>>>>
>>>>>> Because backward compatibility, ipaHBACRuleV2 must contain all
>>>>>> attributes from ipaHBACRule as MAY
>>>>> Not true.
>>>>>
>>>>>> With current approach, when timerule is added to HBAC, we just change
>>>>>> objectclass from 'ipahbacrule' to 'ipahbacrulev2' so we keep all
>>>>>> attributes that was defined in older HBAC. Removing any attrs from
>>>>>> ipaHBACRuleV2 can cause schema violation.
>>>>> Which is perfectly fine.
>>>>>
>>>>>>
>>>>>> I'm not sure if want to handle this in code (removing deprecated
>>>>>> attributes from HBAC entry when timerule is added)
>>>>> We don't have to do anything. If any of the deprecated attributes are
>>>>> present when you change the object class (which they shouldn't
>>>>> anyway), you'll get schema violation, otherwise it will work just
>>>>> fine.
>>>>>
>>>>>> I realized that AccessTime is MUST for 'ipahbacrule', so when
>>>>>> timerule
>>>>>> ('ipahbacrulev2') is removed and somebody deleted accesstime we
>>>>>> have to
>>>>>> add it back.
>>>>> It is MAY. The only MUST attribute is accessRuleType, but that is
>>>>> deprecated as well and should be removed from ipaHBACRuleV2. We only
>>>>> support allow rules, so when timerule is removed, that's the value
>>>>> you set accessRuleType to.
>>>> SSSD does search for HBAC rules by '(objectclass=ipaHBACRule)' filter.
>>>> Changing to ipaHBACRuleV2 means these rules will not be found by older
>>>> SSSD versions and therefore people will experience problems with older
>>>> clients not being able to use new rules even if they would lack time
>>>> component.
>>>>
>>> Older client do not support timerules, so they should not search for
>>> them. HBAC without timerules will be still have 'ipaHBACRule'
>>> objectclass and will work with old clients. Only HBAC with timerules
>>> will have assigned 'ipaHBACRuleV2'
>>>
>>> Martin^2
>> I miss "why" part of "To be able to handle backward compatibility with
>> ease, a new object called ipaHBACRulev2 is introduced. " in the design
>> page. If the reason is the above - old client's should ignore time rules
>> then it has to be mentioned there. Otherwise I don't see a reason to
>> introduce a new object type instead of extending the current.
> 
> How do you want to enforce HBAC rule that have set time from 10 to 14
> everyday? With the same objectclass old clients will allow this HBAC for
> all day. Isn't this CVE?

My point is that the design is missing the explanation.

> 
>>
>>
>> 2. About API and CLI: wasn't there an idea to hide/not provide
>> --icalfile=file.ics and --time=escaped_icalstring  options in the first
>> implementation. So that we can limit the support scope to only a subset
>> of option(the  OPTS part). If arbitrary ical is allowed since the
>> beginning then we are asking for a lot of bugs filed.
>>
> 


-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [DESIGN][UPDATE] Time-Based HBAC Policies

2016-08-26 Thread Petr Vobornik
On 08/26/2016 12:23 PM, Martin Basti wrote:
> 
> 
> On 26.08.2016 12:20, Alexander Bokovoy wrote:
>> On Fri, 26 Aug 2016, Jan Cholasta wrote:
>>> On 26.8.2016 11:55, Martin Basti wrote:
>>>>
>>>>
>>>> On 26.08.2016 11:43, Jan Cholasta wrote:
>>>>> Hi,
>>>>>
>>>>> On 11.8.2016 12:34, Stanislav Laznicka wrote:
>>>>>> Hello,
>>>>>>
>>>>>> I updated the design of the Time-Based HBAC Policies according to the
>>>>>> discussion we led here earlier. Please check the design page
>>>>>> http://www.freeipa.org/page/V4/Time-Based_Account_Policies. The
>>>>>> biggest
>>>>>> changes are in the Implementation and Feature Management sections. I
>>>>>> also added a short How to Use section.
>>>>>
>>>>> 1) Please use the 'ipa' prefix for new attributes: memberTimeRule ->
>>>>> ipaMemberTimeRule
>>>>>
>>>>>
>>>>> 2) Source hosts are deprecated and thus should be removed from
>>>>> ipaHBACRuleV2.
>>>>>
>>>>>
>>>>> 3) Since time rules are defined by memberTimeRule, accessTime should
>>>>> be removed from ipaHBACRuleV2.
>>>>
>>>> ad 2) 3)
>>>>
>>>> Because backward compatibility, ipaHBACRuleV2 must contain all
>>>> attributes from ipaHBACRule as MAY
>>>
>>> Not true.
>>>
>>>>
>>>> With current approach, when timerule is added to HBAC, we just change
>>>> objectclass from 'ipahbacrule' to 'ipahbacrulev2' so we keep all
>>>> attributes that was defined in older HBAC. Removing any attrs from
>>>> ipaHBACRuleV2 can cause schema violation.
>>>
>>> Which is perfectly fine.
>>>
>>>>
>>>>
>>>> I'm not sure if want to handle this in code (removing deprecated
>>>> attributes from HBAC entry when timerule is added)
>>>
>>> We don't have to do anything. If any of the deprecated attributes are
>>> present when you change the object class (which they shouldn't
>>> anyway), you'll get schema violation, otherwise it will work just fine.
>>>
>>>>
>>>> I realized that AccessTime is MUST for 'ipahbacrule', so when timerule
>>>> ('ipahbacrulev2') is removed and somebody deleted accesstime we have to
>>>> add it back.
>>>
>>> It is MAY. The only MUST attribute is accessRuleType, but that is
>>> deprecated as well and should be removed from ipaHBACRuleV2. We only
>>> support allow rules, so when timerule is removed, that's the value
>>> you set accessRuleType to.
>>
>> SSSD does search for HBAC rules by '(objectclass=ipaHBACRule)' filter.
>> Changing to ipaHBACRuleV2 means these rules will not be found by older
>> SSSD versions and therefore people will experience problems with older
>> clients not being able to use new rules even if they would lack time
>> component.
>>
> 
> Older client do not support timerules, so they should not search for
> them. HBAC without timerules will be still have 'ipaHBACRule'
> objectclass and will work with old clients. Only HBAC with timerules
> will have assigned 'ipaHBACRuleV2'
> 
> Martin^2

I miss "why" part of "To be able to handle backward compatibility with
ease, a new object called ipaHBACRulev2 is introduced. " in the design
page. If the reason is the above - old client's should ignore time rules
then it has to be mentioned there. Otherwise I don't see a reason to
introduce a new object type instead of extending the current.


2. About API and CLI: wasn't there an idea to hide/not provide
--icalfile=file.ics and --time=escaped_icalstring  options in the first
implementation. So that we can limit the support scope to only a subset
of option(the  OPTS part). If arbitrary ical is allowed since the
beginning then we are asking for a lot of bugs filed.

-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH 0035] Remove Custodia server keys from LDAP

2016-08-24 Thread Petr Vobornik
On 08/24/2016 12:21 PM, Martin Basti wrote:
> 
> 
> On 24.08.2016 11:25, Christian Heimes wrote:
>> On 2016-08-23 12:42, Petr Vobornik wrote:
>>> On 08/11/2016 04:13 PM, Martin Basti wrote:
>>>>
>>>> On 08.08.2016 16:10, Christian Heimes wrote:
>>>>> The server-del plugin now removes the Custodia keys for encryption and
>>>>> key signing from LDAP.
>>>>>
>>>>> https://fedorahosted.org/freeipa/ticket/6015
>>>>>
>>>>>
>>>> ACK for master
>>>>
>>>> For 4.3, it requires new patch
>>>>
>>>> Martin
>>>>
>>> bump
>> I haven't worked out a patch for 4.3. The server_del plugin in 4.3 is
>> much simpler than in 4.4. It's not possible to hook the clean-up code to
>> server_del like I did for 4.4. I would have to rewrite and redesign the
>> patch completely which I neither have the time nor resources to at the
>> moment.
>>
>> I vote for WONTFIX for 4.3.
> +1

works for me

> 
> Martin^2
>>
>> Christian
>>
-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH 0034] Secure permissions of Custodia server.keys

2016-08-23 Thread Petr Vobornik
On 08/09/2016 01:53 PM, Martin Basti wrote:
> 
> 
> On 08.08.2016 16:09, Christian Heimes wrote:
>> I have split up patch 0032 into two smaller patches. This patch only
>> addresses the server.keys file.
>>
>> Custodia's server.keys file contain the private RSA keys for encrypting
>> and signing Custodia messages. The file was created with permission 644
>> and is only secured by permission 700 of the directory
>> /etc/ipa/custodia. The installer and upgrader ensure that the file
>> has 600.
>>
>> https://bugzilla.redhat.com/show_bug.cgi?id=1353936
>> https://fedorahosted.org/freeipa/ticket/6056
>>
>>
> Pylint is running, please wait ...
> * Module ipapython.secrets.kem
> ipapython/secrets/kem.py:147: [E0602(undefined-variable), newServerKeys] 
> Undefined variable 'os')
> ipapython/secrets/kem.py:148: [E0602(undefined-variable), newServerKeys] 
> Undefined variable 'os')
> * Module ipaserver.install.custodiainstance
> ipaserver/install/custodiainstance.py:77: [E0602(undefined-variable), 
> CustodiaInstance.upgrade_instance] Undefined variable 'stat')
> 
> 
> 

this review looks stuck
-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH 0035] Remove Custodia server keys from LDAP

2016-08-23 Thread Petr Vobornik
On 08/11/2016 04:13 PM, Martin Basti wrote:
> 
> 
> On 08.08.2016 16:10, Christian Heimes wrote:
>> The server-del plugin now removes the Custodia keys for encryption and
>> key signing from LDAP.
>>
>> https://fedorahosted.org/freeipa/ticket/6015
>>
>>
> ACK for master
> 
> For 4.3, it requires new patch
> 
> Martin
> 

bump

-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH 0213] support multiple uid values in slapi-nis users map

2016-08-23 Thread Petr Vobornik
;>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Hello,
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> should we bump requires to slapi-nis-0.56.1 in
>>>>>>>>>>>>>>>>> freeipa.spec?
>>>>>>>>>>>>>>>> No, this is not required. In Fedora we'll submit a
>>>>>>>>>>>>>>>> combined update --
>>>>>>>>>>>>>>>> I've built slapi-nis-0.56.1-1 packages for f24, f25, and
>>>>>>>>>>>>>>>> rawhide already
>>>>>>>>>>>>>>>> but did not submit a Bodhi request.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> How is combined updated related to requires to
>>>>>>>>>>>>>>> slapi-nis-0.56.1?
>>>>>>>>>>>>>>> It will not prevent tu update freeipa without new slapi-nis.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> e.g.
>>>>>>>>>>>>>>> dnf update freeipa-server.
>>>>>>>>>>>>>> An update file in FreeIPA that is proposed by this patch
>>>>>>>>>>>>>> does not affect
>>>>>>>>>>>>>> operation of the older slapi-nis deployment once update is
>>>>>>>>>>>>>> applied.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> Hi,
>>>>>>>>>>>>>
>>>>>>>>>>>>> Is '%first' returning the first value of the attribute 'uid' ?
>>>>>>>>>>>>> If there are several values (canonical, alias,... ), does
>>>>>>>>>>>>> the order matters ?
>>>>>>>>>>>> We insert the canonical one first and it seems that 389-ds
>>>>>>>>>>>> does not
>>>>>>>>>>>> change the order, at least in my tests. You can see the
>>>>>>>>>>>> output in the
>>>>>>>>>>>> bug https://bugzilla.redhat.com/show_bug.cgi?id=1361123#c2
>>>>>>>>>>>
>>>>>>>>>>> From ldap pov
>>>>>>>>>>> (https://tools.ietf.org/html/rfc4511#section-4.1.7) the order
>>>>>>>>>>> of the values is not preserved.
>>>>>>>>>>> I think it is a bit risky to rely on a specific order
>>>>>>>>>>> especially with complex updates (adding several values in a
>>>>>>>>>>> single mod, replace) and replication.
>>>>>>>>>>> would it help to add canonical value with a subtype (e.g.
>>>>>>>>>>> 'uid;canonical: ') ?
>>>>>>>>>> Not sure how what you are mention is relevant here. We talk about
>>>>>>>>>> slapi-nis map cache entries which are not modified, replaced or
>>>>>>>>>> replicated anywhere by anything other than slapi-nis itself. When
>>>>>>>>>> entries are changed by slapi-nis, they are regenerated from
>>>>>>>>>> scratch.
>>>>>>>>>>
>>>>>>>>>> Your worries do not apply here.
>>>>>>>>> ok.
>>>>>>>>> I understand my mistake, I was thinking the compat entry had a
>>>>>>>>> associated real entry in ldap and this real entry had two uid
>>>>>>>>> values.
>>>>>>>>>
>>>>>>>>
>>>>>>>> So, could you provide ack thierry?
>>>>>>>>
>>>>>>>> Martin
>>>>>>>
>>>>>>> Sure but I would have to test first :-)
>>>>>>>
>>>>>>> Alexander, how can I test this ?
>>>>>> slapi-nis 0.56.1-1 packages are available in koji for f24-f26:
>>>>>> http://koji.fedoraproject.org/koji/packageinfo?packageID=6609
>>>>>
>>>>> Thanks Alexander. So to test this change is there an other way
>>>>> (simpler) than setting up AD/trust ?
>>>> I don't think so. We don't have UPNs in FreeIPA on the level of
>>>> identity
>>>> lookups and we don't allow to lookup identity by email, so you are left
>>>> with using a proper AD trust and UPN suffixes in AD forest.
>>> Attached is an updated patch that adds versioned require of slapi-nis
>>> which supports the feature.
>>>
>>>
>>
>> Thanks to Lenka, I was able to test the patch with AD trust and a UPN
>> suffix.
>>
>> The tests looks good as 'getent' on a AD user returns user@ADdomain.
>>
>> Now if I remove the config change in "cn=users,cn=Schema
>> Compatibility,cn=plugins,cn=config", I get the same results i.e:
>> getent passwd upnu...@upnsuffix.com
>> upnu...@dom-221.idm.lab.eng.brq.redhat.com:*:1965201133:1965201133:UPN
>> User:/:
>>
>>
>> How can I check slapi-nis gets the first value ?
>>
>> thanks
>> thierry
> acceptance is now completed (successfully).  ACK
> 

bump

so ACKed ab's 213-1 fixes https://fedorahosted.org/freeipa/ticket/6138 ?

-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH] 0004 Added support for authentication with user certificate

2016-08-16 Thread Petr Vobornik
On 08/16/2016 10:17 AM, Jan Cholasta wrote:
> On 12.8.2016 15:02, Petr Vobornik wrote:
>> On 08/12/2016 02:54 PM, Tibor Dudlak wrote:
>>> Hi,
>>>
>>> I have edited my previous patch.
>>>
>>> On Thu, Aug 11, 2016 at 11:52 AM, Jan Cholasta <jchol...@redhat.com
>>> <mailto:jchol...@redhat.com>> wrote:
>>>
>>> Hi,
>>>
>>> On 11.8.2016 09:55, Tibor Dudlak wrote:
>>>
>>> Hi,
>>>
>>> ...
>>>
>>>
>>> +class login_x509(login_kerberos, KerberosSession, HTTP_Status):
>>> +key = '/session/login_x509'
>>>
>>> login_kerberos already subclasses KerberosSession and
>>> HTTP_Status, no need
>>> to do it again here. In fact, it would be best to split off the
>>> bussiness
>>> logic from login_kerberos into a separate class and inherit both
>>> login_kerberos and login_x509 from it:
>>>
>>>  class KerberosLogin(Backend, KerberosSession, HTTP_Status):
>>>  def _on_finalize(self):
>>>  ...
>>>
>>>  def __call__(self, ...):
>>>  ...
>>>
>>>  class login_kerberos(KerberosLogin):
>>>  key = '/session/login_kerberos'
>>>
>>>  class login_x509(KerberosLogin):
>>>  key = '/session/login_x509'
>>>
>>> Honza
>>>
>>> --
>>> Jan Cholasta
>>>
>>>
>>> Thank jcholast for review, it should be all right now.
>>>
>>> -- 
>>> Tibor Dudlák
>>> Intern - Identity management Special Projects
>>> Red Hat
>>>
>>
>> Tibor, please reuse the original thread and patch number in each patch
>> iteration. But append new patch version. E.g.
>> freeipa-ddudla-0003-2-Added...
>>
>> Starting new thread for each patch revision makes it hard to track.
> 
> +1
> 
> As far as the patch is concerned, LGTM.
> 

Anyway, I'd split the patch into two pieces:

1. the python part
2. the webui plugin + related conf

Reason: there is a wide agreement that #1 will be push. It's not about #2.

-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH] 0004 Added support for authentication with user certificate

2016-08-12 Thread Petr Vobornik
On 08/12/2016 02:54 PM, Tibor Dudlak wrote:
> Hi,
> 
> I have edited my previous patch.
> 
> On Thu, Aug 11, 2016 at 11:52 AM, Jan Cholasta <jchol...@redhat.com 
> <mailto:jchol...@redhat.com>> wrote:
> 
> Hi,
> 
> On 11.8.2016 09:55, Tibor Dudlak wrote:
> 
> Hi,
> 
> ...
> 
> 
> +class login_x509(login_kerberos, KerberosSession, HTTP_Status):
> +key = '/session/login_x509'
> 
> login_kerberos already subclasses KerberosSession and HTTP_Status, no need
> to do it again here. In fact, it would be best to split off the bussiness
> logic from login_kerberos into a separate class and inherit both
> login_kerberos and login_x509 from it:
> 
>  class KerberosLogin(Backend, KerberosSession, HTTP_Status):
>  def _on_finalize(self):
>  ...
> 
>  def __call__(self, ...):
>  ...
> 
>  class login_kerberos(KerberosLogin):
>  key = '/session/login_kerberos'
> 
>  class login_x509(KerberosLogin):
>  key = '/session/login_x509'
> 
> Honza
> 
> -- 
> Jan Cholasta
> 
> 
> Thank jcholast for review, it should be all right now.
> 
> -- 
> Tibor Dudlák
> Intern - Identity management Special Projects
> Red Hat
> 

Tibor, please reuse the original thread and patch number in each patch
iteration. But append new patch version. E.g. freeipa-ddudla-0003-2-Added...

Starting new thread for each patch revision makes it hard to track.
-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH] 0001 Added new authentication method

2016-08-11 Thread Petr Vobornik
On 08/11/2016 07:21 PM, Martin Basti wrote:
> 
> 
> On 11.08.2016 18:57, Pavel Vomacka wrote:
>>
>>
>> On 08/11/2016 02:00 PM, Petr Vobornik wrote:
>>> On 08/11/2016 10:54 AM, Alexander Bokovoy wrote:
>>>> On Thu, 11 Aug 2016, Jan Cholasta wrote:
>>>>> On 4.8.2016 17:27, Jan Pazdziora wrote:
>>>>>> On Wed, Aug 03, 2016 at 10:29:52AM +0300, Alexander Bokovoy wrote:
>>>>>>> Got it. One thing I would correct, though, -- don't use
>>>>>>> kadmin.local, we
>>>>>>> do support setting ok_as_delegate on the service principals via IPA
>>>>>>> CLI:
>>>>>>> $ ipa service-mod --help |grep -A1 ok-as-delegate
>>>>>>> --ok-as-delegate=BOOL
>>>>>>>Client credentials may be delegated to the
>>>>>>> service
>>>>>> I've tried
>>>>>>
>>>>>>  ipa service-mod --ok-as-delegate=True HTTP/$(hostname)
>>>>>>
>>>>>> but that does not seem to have the same effect as
>>>>>>
>>>>>>  modprinc +ok_to_auth_as_delegate HTTP/ipa.example.test
>>>>>>
>>>>>> -- obtaining the delegated certificated fails.
>>>>> That's because ok_as_delegate and ok_to_auth_as_delegate are different
>>>>> flags.
>>>> Right. The following patch adds ok_to_auth_as_delegate to the service
>>>> principal.
>>>>
>>>> I haven't added any tickets to it yet.
>>>>
>>>>
>>> This might deserve also nice Web UI checkbox similar to "Trusted for
>>> delegation". CCing Pavel.
>>>
>> Here is patch with new checkbox. It is without ticket in commit message so 
>> once we will have the ticket I will send another patch witch updated commit 
>> message.
> 
> https://fedorahosted.org/freeipa/newticket
> 
> ;-)

It's prerequisite for https://fedorahosted.org/freeipa/ticket/5764 so we
might use that.
> 
>>
>>
>>
> 
> 
> 


-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH] 0001 Added new authentication method

2016-08-11 Thread Petr Vobornik
On 08/11/2016 10:54 AM, Alexander Bokovoy wrote:
> On Thu, 11 Aug 2016, Jan Cholasta wrote:
>> On 4.8.2016 17:27, Jan Pazdziora wrote:
>>> On Wed, Aug 03, 2016 at 10:29:52AM +0300, Alexander Bokovoy wrote:
>>>>
>>>> Got it. One thing I would correct, though, -- don't use
>>>> kadmin.local, we
>>>> do support setting ok_as_delegate on the service principals via IPA
>>>> CLI:
>>>> $ ipa service-mod --help |grep -A1 ok-as-delegate
>>>> --ok-as-delegate=BOOL
>>>>   Client credentials may be delegated to the
>>>> service
>>>
>>> I've tried
>>>
>>> ipa service-mod --ok-as-delegate=True HTTP/$(hostname)
>>>
>>> but that does not seem to have the same effect as
>>>
>>> modprinc +ok_to_auth_as_delegate HTTP/ipa.example.test
>>>
>>> -- obtaining the delegated certificated fails.
>>
>> That's because ok_as_delegate and ok_to_auth_as_delegate are different
>> flags.
> Right. The following patch adds ok_to_auth_as_delegate to the service
> principal.
> 
> I haven't added any tickets to it yet.
> 
> 

This might deserve also nice Web UI checkbox similar to "Trusted for
delegation". CCing Pavel.

-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH 0060] Add --force-join option to ipa-replica-install

2016-08-09 Thread Petr Vobornik
On 08/09/2016 04:18 PM, Martin Basti wrote:
> 
> 
> On 09.08.2016 16:07, Stanislav Laznicka wrote:
>> https://fedorahosted.org/freeipa/ticket/6183
>>
>>
>>
> Didn't we agreed that --force-join should be always used (without extra 
> replica-install option)

+1

> 
> Martin^2
> 
-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH 0214] Support schema files for external plugins

2016-08-08 Thread Petr Vobornik
On 08/08/2016 12:26 PM, Alexander Bokovoy wrote:
> On Mon, 08 Aug 2016, Alexander Bokovoy wrote:
>> Hi!
>>
>> Attached patch is what is needed to allow external plugins for FreeIPA
>> framework to be functional if they need to extend a schema.
>>
>> The idea is that we would have a separate directory as
>> /usr/share/ipa/schema.d and will allow to use schema (*.ldif) files from
>> it and its subdirectories during install and upgrade stages.
>>
>> Without the patch only selected schema files from /usr/share/ipa are
>> used during install and upgrade. This leads to a failure to install IPA
>> server (or upgrade it) if a new plugin is added. If plugin defines
>> managed permissions, upgrade tool will generate ACIs which will fail to
>> be inserted into LDAP store due to references to missing attributes and
>> object classes.
>>
>> The patch adds a directory to be installed and a helper utility that
>> loads files from the directory and adds them to the list of schema files
>> used during update of dsinstance and upgrade of the server.
>>
>> With this patch I'm successfully managed to make FleetCommander
>> integration plugin completely independent of FreeIPA.
> Patch attached now. ;)
> 

I'll assume that we want to target 4.4.x therefore it can be pushed to
master, right? I.e. no need for creating ipa-4-4 branch atm.

Reasoning is that currently F24 has 4.3.x. F25 will most likely have
4.4.x because 4.5 is still in planning.

-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH] 965 ca-less tests: fix getting cert in pem format from nssdb

2016-08-06 Thread Petr Vobornik
On 08/06/2016 12:32 PM, Petr Vobornik wrote:
> usage of ipautil.run in  get_pem methond of ca-less tests was not
> refactored when the ipautil.run was refactored in
> 099cf98307d4b2f0ace5d5e28754f264808bf59d
> 
> This results in failure of all CA-less test (probably).
> 
> Patch is untested though.
> 
> https://fedorahosted.org/freeipa/ticket/6177
> 

Original patch doesn't fix the issue. Main culprit is that output is
captured only if capture_output=True is passed to ipautil.run

Previous patch therefore just replaced old usage to new usage.

Attaching fixed path.
-- 
Petr Vobornik
From 3d66938af86c7108e0a20daf0437e0c3577bad3d Mon Sep 17 00:00:00 2001
From: Petr Vobornik <pvobo...@redhat.com>
Date: Sat, 6 Aug 2016 12:25:57 +0200
Subject: [PATCH] ca-less tests: fix getting cert in pem format from nssdb

usage of ipautil.run in  get_pem methond of ca-less tests was not
refactored when the ipautil.run was refactored in
099cf98307d4b2f0ace5d5e28754f264808bf59d

This results in failure of all CA-less test.

https://fedorahosted.org/freeipa/ticket/6177
---
 ipatests/test_integration/test_caless.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/ipatests/test_integration/test_caless.py b/ipatests/test_integration/test_caless.py
index 667e2b3b1d91f967b32fabdb7e472886bbdf79d7..c9d90331bd2658b7164e6a9e70f07bbc8960ff07 100644
--- a/ipatests/test_integration/test_caless.py
+++ b/ipatests/test_integration/test_caless.py
@@ -279,10 +279,10 @@ class CALessBase(IntegrationTest):
 
 @classmethod
 def get_pem(cls, nickname):
-pem_cert, _stderr, _returncode = ipautil.run(
+result = ipautil.run(
 ['certutil', '-L', '-d', 'nssdb', '-n', nickname, '-a'],
-cwd=cls.cert_dir)
-return pem_cert
+cwd=cls.cert_dir, capture_output=True)
+return result.output
 
 def verify_installation(self):
 """Verify CA cert PEM file and LDAP entry created by install
-- 
2.5.5

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

[Freeipa-devel] [PATCH] 965 ca-less tests: fix getting cert in pem format from nssdb

2016-08-06 Thread Petr Vobornik
usage of ipautil.run in  get_pem methond of ca-less tests was not
refactored when the ipautil.run was refactored in
099cf98307d4b2f0ace5d5e28754f264808bf59d

This results in failure of all CA-less test (probably).

Patch is untested though.

https://fedorahosted.org/freeipa/ticket/6177
-- 
Petr Vobornik
From 390731b873b3e47fe26b5ccd59dca39b8afeecd3 Mon Sep 17 00:00:00 2001
From: Petr Vobornik <pvobo...@redhat.com>
Date: Sat, 6 Aug 2016 12:25:57 +0200
Subject: [PATCH] ca-less tests: fix getting cert in pem format from nssdb

usage of ipautil.run in  get_pem methond of ca-less tests was not
refactored when the ipautil.run was refactored in
099cf98307d4b2f0ace5d5e28754f264808bf59d

This results in failure of all CA-less test.

https://fedorahosted.org/freeipa/ticket/6177
---
 ipatests/test_integration/test_caless.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ipatests/test_integration/test_caless.py b/ipatests/test_integration/test_caless.py
index 667e2b3b1d91f967b32fabdb7e472886bbdf79d7..d443df0fe0d761c4b82145d5550aaf24df4acdb5 100644
--- a/ipatests/test_integration/test_caless.py
+++ b/ipatests/test_integration/test_caless.py
@@ -279,10 +279,10 @@ class CALessBase(IntegrationTest):
 
 @classmethod
 def get_pem(cls, nickname):
-pem_cert, _stderr, _returncode = ipautil.run(
+result = ipautil.run(
 ['certutil', '-L', '-d', 'nssdb', '-n', nickname, '-a'],
 cwd=cls.cert_dir)
-return pem_cert
+return result.output
 
 def verify_installation(self):
 """Verify CA cert PEM file and LDAP entry created by install
-- 
2.5.5

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH] 0002 Added support for authentication with user certificate

2016-08-05 Thread Petr Vobornik
On 08/05/2016 02:57 PM, Tibor Dudlak wrote:
> Hi,
> 
> I have extended my previous patch for authentication with user 
> certificate/smartcard. This patch includes patches and plugin described here: 
> http://www.freeipa.org/page/V4/External_Authentication/Setup
> Page also contains steps to configure and test this feature. Once this patch 
> is 
> merged and released we will simplify this page to not confuse customers.
> Addressing ticket: https://fedorahosted.org/freeipa/ticket/5764
> 

Let's assume that we will go with this approach and not separate RPM.

1. ipa.conf version needs to be bumped

2. Do not put the web ui plugin in src/freeipa/plugins dir. That is a
dir for core UI plugins. This one is sort of hybrid - basically a third
party plugin added to core package  but enabled as third party because
the feature is experimental.

Create rather a new dir for that. E.g. plugins.d as Alexander suggested
->  freeipa/install/ui/src/plugins.d/cert_auth/cert_auth.js

3. unrelated and "alternative solution"  comments needs to be removed
from the UI plugin. They were added to the example plugin
https://pvoborni.fedorapeople.org/plugins/loginauth/loginauth.js mostly
to help you with the development.

4. Add comment to freeipa.spec.in describing what the plugin is and why
it is put there this way.

5. The plugin itself deserves better description as well. Right now
there is the general description.

6. I have not tried it, but make sure that it passes jslint (`jsl -conf
jsl.conf`) Easiest may be to use temp(i.e. do not include it here)
jsl.conf e.g.: https://pvoborni.fedorapeople.org/plugins/loginauth/jsl.conf

-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCHES] Coverity fixes

2016-08-05 Thread Petr Vobornik
On 07/28/2016 01:01 PM, Martin Basti wrote:
> 
> 
> On 25.07.2016 11:46, Simo Sorce wrote:
>> The attached patches fix some minor issues found by coverity, and pull
>> in other fixes released by the asn1c project.
>>
>> Simo.
>>
>>
>>
> I cannot build RPMS with this patch, is there any missing build dependency?
> 
> /bin/sh ./libtool  --tag=CC   --mode=link gcc  -Wall -Wshadow 
> -Wstrict-prototypes -Wpointer-arith -Wcast-align 
> -Werror-implicit-function-declaration  -O2 -g -pipe -Wall 
> -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions 
> -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches 
> -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -m64 -mtune=generic -g -O2 
> -Wall 
> -Wextra -Wformat-security -Wno-unused-parameter -Wno-sign-compare 
> -Wno-missing-field-initializers   -Wl,-z,relro 
> -specs=/usr/lib/rpm/redhat/redhat-hardened-ld  -o ipa-getkeytab 
> ipa-getkeytab.o 
> ipa-client-common.o ipa_krb5.o ../asn1/libipaasn1.la -lkrb5 -lk5crypto 
> -lcom_err 
> -llber -lldap -lsasl2 -lpopt  -lini_config -lbasicobjects -lref_array 
> -lcollection  -lini_config -lini_config
> libtool: link: gcc -Wall -Wshadow -Wstrict-prototypes -Wpointer-arith 
> -Wcast-align -Werror-implicit-function-declaration -O2 -g -pipe -Wall 
> -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions 
> -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches 
> -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -m64 -mtune=generic -g -O2 
> -Wall 
> -Wextra -Wformat-security -Wno-unused-parameter -Wno-sign-compare 
> -Wno-missing-field-initializers -Wl,-z -Wl,relro 
> -specs=/usr/lib/rpm/redhat/redhat-hardened-ld -o ipa-getkeytab 
> ipa-getkeytab.o 
> ipa-client-common.o ipa_krb5.o ../asn1/.libs/libipaasn1.a -lkrb5 -lk5crypto 
> -lcom_err -llber -lldap -lsasl2 -lpopt -lbasicobjects -lref_array 
> -lcollection 
> -lini_config
> ../asn1/.libs/libipaasn1.a(constr_CHOICE.o): In function `CHOICE_decode_uper':
> /root/freeipa/rpmbuild/BUILD/freeipa-4.4.0/asn1/asn1c/constr_CHOICE.c:897: 
> undefined reference to `uper_open_type_get'
> ../asn1/.libs/libipaasn1.a(constr_CHOICE.o): In function `CHOICE_encode_uper':
> /root/freeipa/rpmbuild/BUILD/freeipa-4.4.0/asn1/asn1c/constr_CHOICE.c:982: 
> undefined reference to `uper_open_type_put'
> ../asn1/.libs/libipaasn1.a(constr_SEQUENCE.o): In function 
> `SEQUENCE_handle_extensions':
> /root/freeipa/rpmbuild/BUILD/freeipa-4.4.0/asn1/asn1c/constr_SEQUENCE.c:1285: 
> undefined reference to `uper_open_type_put'
> ../asn1/.libs/libipaasn1.a(constr_SEQUENCE.o): In function 
> `SEQUENCE_decode_uper':
> /root/freeipa/rpmbuild/BUILD/freeipa-4.4.0/asn1/asn1c/constr_SEQUENCE.c:1187: 
> undefined reference to `uper_open_type_get'
> /root/freeipa/rpmbuild/BUILD/freeipa-4.4.0/asn1/asn1c/constr_SEQUENCE.c:1203: 
> undefined reference to `uper_open_type_skip'
> collect2: error: ld returned 1 exit status
> 
> Martin^2
> 

Bumping. Was it temporary issue or issue in the patch?

-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH] 0090, 0092..0094 cert-show: show subject alternative names

2016-08-04 Thread Petr Vobornik
On 07/22/2016 07:13 AM, Fraser Tweedale wrote:
> On Tue, Jul 19, 2016 at 08:50:34AM +0200, Jan Cholasta wrote:
>> Hi,
>>
>> On 14.7.2016 13:44, Fraser Tweedale wrote:
>>> Hi all,
>>>
>>> The attached patch includes SANs in cert-show output.  If you have
>>> certs with esoteric altnames (especially any that are more than just
>>> ASN.1 string types), please test with those certs.
>>>
>>> https://fedorahosted.org/freeipa/ticket/6022
>>
>> I think it would be better to have a separate attribute for each supported
>> SAN type rather than cramming everything into subject_alt_name. That way if
>> you care only about a single specific type you won't have to go through all
>> the values and parse them. Also it would allow you to use param types
>> appropriate to the SAN types (DNSNameParam for DNS names, Principal for
>> principal names, etc.)
>>
>> Nitpick: please don't mix moving existing stuff and adding new stuff in a
>> single patch.
>>
> Updated patches attached.
> 
> Patches 0092..0094 are refactors and bugfixes.
> Patch 0090-2 is the main feature (depends on 0092..0094).
> 
> Thanks,
> Fraser
> 

bump for review
-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH 0057] Don't show part of warning containing --force-ntpd in replica install

2016-08-03 Thread Petr Vobornik
On 07/13/2016 12:36 PM, Stanislav Laznicka wrote:
> On 07/13/2016 09:51 AM, Petr Vobornik wrote:
>> On 07/13/2016 08:26 AM, Stanislav Laznicka wrote:
>>> On 07/12/2016 08:44 AM, Stanislav Laznicka wrote:
>>>> On 07/11/2016 04:27 PM, Petr Vobornik wrote:
>>>>> On 07/11/2016 01:23 PM, Stanislav Laznicka wrote:
>>>>>> https://fedorahosted.org/freeipa/ticket/6046
>>>>>>
>>>>>>
>>>>>>
>>>>> Isn't the bug about something else?
>>>>>
>>>>> The issue was that ipa-replica-install doesn't have --force-ntpd
>>>>> option.
>>>>> It is an option of ipa-client-install which is run from replica
>>>>> installer.
>>>>>
>>>>> The unattended mode is unrelated.
>>>> My understanding is that the bug says that '--force-ntpd' option
>>>> should not be shown when ipa-client-install is run during replica
>>>> installation.
>>>>
>>>> During replica installation, the ipa-client-install script is run with
>>>> the '--unattended' flag in the 'ensure_enrolled()' function. Being a
>>>> separate script, there's not many options on how to pass the
>>>> information not to show the message to ipa-client-install. Using the
>>>> already used flag to get rid of the message seemed easiest to me.
>>>> Introducing a new 'hidden' flag (like '--from-replica'), on the other
>>>> hand, seems a bit harsh.
>>>>
>>> Just to throw it out there - it's possible that the '--force-join'
>>> client option would also appear as a hint from the client install script
>>> (during replica installation). Should this also be muted somehow? To me,
>>> it seems reasonable to rather add it as an argument to
>>> ipa-replica-install to pass it to the client install script.
>>>
>> IMO client installation initiated from replica needs to have a special
>> option(hidden in help) similar to --on-server (or what's its name). E.g.
>> the name can be --replica-install. Maybe --on-server can be used but it
>> may have other implication which might not be valid for this use case.
>>
>> Anything else are just workarounds. Imagine that admin runs
>> ipa-client-install with --unattended or --force-join. He would then not
>> get the message as now.
> 

Reviving thread to get other opinion.

> The --on-master option won't do here as it seems that the client would
> require some IPA pre-configuration for successful install. A new option
> will have to be created, then.

I'm for new "hidden" option.

> 
> As I was trying to point out, the situation about --force-join is a bit
> different. The option again would be shown and is not available in
> ipa-replica-install. I think it should be available to allow direct
> replica installation even when previous installation failed/left some
> mess on the master (ofc the user could run `ipa-replica-manage del
>  --cleanup` on the master instead).
> 

That could work but imho is out of scope of this ticket.
-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH] webui: Fix coverity bugs

2016-08-01 Thread Petr Vobornik
owserWidget.js
>> @@ -135,7 +135,7 @@ widgets.APIBrowserWidget = declare([Stateful,
>> Evented], {
>> groups = this._get_params(parts[0]);
>> }
>>
>> -if (filter) {
>> +if (filter && groups) {
>> filter = filter.toLowerCase();
>> var new_groups = [];
>> for (var i=0,l=groups.length; i<l; i++) {
>> @@ -153,10 +153,10 @@ widgets.APIBrowserWidget = declare([Stateful,
>> Evented], {
>> new_groups.push(groups[i]);
>> }
>> }
>> -return new_groups;
>> -} else {
>> -return groups;
>> +groups = new_groups;
>> }
>> +
>> +return groups;
>> },
>>
>> /**
>> -- 
>> 2.5.5
>>
> ACK

ACK

> 
> 
>> From 3d63ca1d5cb7a7b84cf20c26d4b1ea5b657c44c4 Mon Sep 17 00:00:00 2001
>> From: Pavel Vomacka <pvoma...@redhat.com>
>> Date: Tue, 26 Jul 2016 12:03:28 +0200
>> Subject: [PATCH 11/13] Coverity - opens dialog which might not be created
>>
>> Check whether dialog object is created before opening it.
>> ---
>> install/ui/src/freeipa/search.js | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/install/ui/src/freeipa/search.js
>> b/install/ui/src/freeipa/search.js
>> index
>> 25f21e70db170daf0d45a6862ee9adb528ad03bc..fee1bc7523d6afdb3e2b23db2833a415febb85ec
>> 100644
>> --- a/install/ui/src/freeipa/search.js
>> +++ b/install/ui/src/freeipa/search.js
>> @@ -221,7 +221,7 @@ IPA.search_facet = function(spec, no_init) {
>> that.show_remove_dialog = function() {
>>
>> var dialog = that.create_remove_dialog();
>> -dialog.open();
>> +if (dialog) dialog.open();
>> };
>>
>> that.find = function() {
>> -- 
>> 2.5.5
>>
> ACK


ACK but question is whether we should laso log to console that dialog is
not defined because it just hides an issue which may be harder to debug.


> 
>> From 7819293fc546de31cc5eea246242742af3be094e Mon Sep 17 00:00:00 2001
>> From: Pavel Vomacka <pvoma...@redhat.com>
>> Date: Tue, 26 Jul 2016 13:07:30 +0200
>> Subject: [PATCH 12/13] Coverity - accessing attribute of variable
>> which can
>> point to null
>>
>> Added check whether variable is pointing to null or not.
>> ---
>> install/ui/src/freeipa/widget.js | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/install/ui/src/freeipa/widget.js
>> b/install/ui/src/freeipa/widget.js
>> index
>> 43804c5ea524ca741017d02f6e12ccf60d50b5df..1f61ce7341b1b8e13d4df5acea1f8901a63a290a
>> 100644
>> --- a/install/ui/src/freeipa/widget.js
>> +++ b/install/ui/src/freeipa/widget.js
>> @@ -4938,7 +4938,7 @@ IPA.combobox_widget = function(spec) {
>> var value = that.list.val();
>> var option = $('option[value="'+value+'"]', that.list);
>> var next = option.next();
>> -if (!next.length) return;
>> +if (!next || !next.length) return;
>> that.select(next.val());
>> };
>>
>> @@ -4946,7 +4946,7 @@ IPA.combobox_widget = function(spec) {
>> var value = that.list.val();
>> var option = $('option[value="'+value+'"]', that.list);
>> var prev = option.prev();
>> -if (!prev.length) return;
>> +if (!prev || !prev.length) return;
>> that.select(prev.val());
>> };
>>
>> -- 
>> 2.5.5
>>
> ACK

ACK, but IMO the situation cannot happen. .next() and .prev() should not
return null ever.

> 
>> From 3ba5110fa8b2255b83fa3e7a4135ec33b85a7fd8 Mon Sep 17 00:00:00 2001
>> From: Pavel Vomacka <pvoma...@redhat.com>
>> Date: Fri, 29 Jul 2016 10:13:21 +0200
>> Subject: [PATCH 13/13] Coverity - null pointer dereference
>>
>> Add check which protect from calling method of null.
>> ---
>> install/ui/src/freeipa/widgets/APIBrowserWidget.js | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/install/ui/src/freeipa/widgets/APIBrowserWidget.js
>> b/install/ui/src/freeipa/widgets/APIBrowserWidget.js
>> index
>> 18773536d3587cdeb9e5fecedcc5e42c05bfe120..2164df2f5ffa00edf9ac41fd4cf6254f6d4eb9a3
>> 100644
>> --- a/install/ui/src/freeipa/widgets/APIBrowserWidget.js
>> +++ b/install/ui/src/freeipa/widgets/APIBrowserWidget.js
>> @@ -264,7 +264,7 @@ widgets.APIBrowserWidget = declare([Stateful,
>> Evented], {
>> this.list_w.select(item);
>>
>> // set item
>> -widget.set('item', item);
>> +if (widget) widget.set('item', item);
>> this.set('current', {
>> item: item,
>> type: type,
>> -- 
>> 2.5.5
>>
> ACK
> 

Does it fix the issue? There is a line before this one which also uses
`widget`

  if (!widget.el) widget.render();

maybe we miss `return;` in:

} else {
IPA.notify("Invalid type", 'error');
this.show_default();
}





-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


[Freeipa-devel] Announcing FreeIPA 4.3.2

2016-07-24 Thread Petr Vobornik
_test: Rename exception instance before working with it
* radiusproxy plugin: Use str(error) rather than error.message
* xmlrpc_test: Expect bytes rather than strings for binary attributes
* ipalib.rpc: Send base64-encoded data as string under Python 3
* range plugin tests: Use bytes with MockLDAP under Python 3
* radiusproxy plugin tests: Expect bytes, not text, for ipatokenradiussecret
* certprofile plugin: Use binary mode for file with binary data
* test_add_remove_cert_cmd: Use bytes for base64.b64encode()
* Switch /usr/bin/ipa to Python 3
* Fix remaining relative import and enable Pylint check
* ipalib.cli: Improve reporting of binary values in the CLI
* test_cert_plugin: Encode 'certificate' for comparison with
'usercertificate'
* ipaldap: Keep attribute names as text, not bytes
* ipapython.secrets.kem: Use ConfigParser from six.moves
* test_topology_plugin: Don't rely on order of an attribute's values
* test_rpcserver: Expect updated error message under Python 3
* ipaplatform.redhat: Use bytestrings when calling rpm.so for version
comparison
* test_ipaserver.test_ldap: Use bytestrings for raw LDAP values
* ipaldap: Convert dict items to list before iterating
* test_ipaserver.test_ldap: Adjust tests to Python 3's KeyView

=== Petr Voborník (2) ===
* mod_auth_gssapi: enable unique credential caches names
* Become IPA 4.3.2

=== Petr Špaček (30) ===
* Remove function ipapython.ipautil.host_exists()
* Extend installers with --forward-policy option
* Move automatic empty zone list into ipapython.dnsutil and make it reusable
* Add assert_absolute_dnsname() helper to ipapython.dnsutil
* Move function is_auto_empty_zone() into ipapython.dnsutil
* Use shared sanity check and tests ipapython.dnsutil.is_auto_empty_zone()
* Add function ipapython.dnsutil.inside_auto_empty_zone()
* Auto-detect default value for --forward-policy option in installers
* DNS: Fix upgrade - master to forward zone transformation
* DNS installer: accept --auto-forwarders option in unattended mode
* Batch command: avoid accessing potentially undefined context.principal
* Move check_zone_overlap() from ipapython.ipautil to ipapython.dnsutil
* Use root_logger for verify_host_resolvable()
* Move IP address resolution from ipaserver.install.installutils to
ipapython.dnsutil
* Turn verify_host_resolvable() into a wrapper around ipapython.dnsutil
* Add ipaDNSVersion option to dnsconfig* commands and use new attribute
* DNS upgrade: separate backup logic to make it reusable
* Add function ipapython.dnsutil.related_to_auto_empty_zone()
* DNS upgrade: change forwarding policy to = only for conflicting
forward zones
* DNS upgrade: change global forwarding policy in LDAP to "only" if
private IPs are used
* DNS upgrade: change global forwarding policy in named.conf to "only"
if private IPs are used
* DNS: Warn if forwarding policy conflicts with automatic empty zones
* DNS: Fix realm domains integration with DNS zone add.
* client: Share validator and domain name normalization with server install
* DNS: Fix tests for realm domains integration with DNS zone add
* client-install: do not fail if DNS times out during DNS update generation
* Use NSS for name->resolution in IPA installer
* DNS: Remove unnecessary DNS check from installer
* Remove unused is_local(), interface, and defaultnet from CheckedIPAddress
* Fix internal errors in host-add and other commands caused by DNS
resolution

=== Stanislav Laznicka (9) ===
* replica-manage: fail nicely when DM psswd required
* ipa-replica-manage refactoring
* abort-clean/list/clean-ruv now work for both suffixes
* Moved password check from clean_dangling_ruv
* Fix to clean-dangling-ruv for single CA topologies
* Added pyusb as a dependency
* Deprecated the domain-level option in ipa-server-install
* fixes premature sys.exit in ipa-replica-manage del
* Remove dangling RUVs even if replicas are offline

=== Thierry Bordaz (1) ===
* Make sure ipapwd_extop takes precedence over passwd_modify_extop

-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

[Freeipa-devel] [DRAFT] FreeIPA 4.3.2 release notes

2016-07-21 Thread Petr Vobornik
Hi all,

this is a draft of release notes for upcoming 4.3.2 release
- http://www.freeipa.org/page/Releases/4.3.2

Comments/updates welcome!

Regards,
-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH] 963 unite log file name of ipa-ca-install

2016-07-21 Thread Petr Vobornik
On 07/21/2016 05:47 PM, Martin Babinsky wrote:
> On 07/21/2016 05:22 PM, Petr Vobornik wrote:
>> On 07/19/2016 09:27 AM, Petr Vobornik wrote:
>>> On 07/19/2016 08:01 AM, Jan Cholasta wrote:
>>>> Hi,
>>>>
>>>> On 18.7.2016 18:50, Florence Blanc-Renaud wrote:
>>>>> On 07/15/2016 04:29 PM, Petr Vobornik wrote:
>>>>>> ipa-ca-install said that it used
>>>>>>   /var/log/ipareplica-ca-install.log
>>>>>> but in fact it used
>>>>>>   /var/log/ipaserver-ca-install.log
>>>>>>
>>>>>> This patch unites it to ipaserver-ca-install.log
>>>>>>
>>>>>> It was chosen because ipa-ca-install can be also used on
>>>>>> master on CA-less -> CA conversion.
>>>>>>
>>>>>> Term "server" is valid for both master and replica.
>>>>>>
>>>>>> https://fedorahosted.org/freeipa/ticket/6088
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>> Looks good to me.
>>>>> Ack
>>>>
>>>> Does not look so good to me, "ipareplica-ca-install.log" is in fact the
>>>> original file name used since ipa-ca-install was introduced (in commit
>>>> 8a32bb3746802a29b2655e4ad2cbbba8481e1eaf), so why the switch to
>>>> "ipaserver-ca-install.log"?
>>>>
>>>> Honza
>>>>
>>>
>>> Ideally it would be ipa-ca-install.log but for backwards compatibility,
>>> let's stick with one which we have. AFAIK the framework(run_script
>>> methodú doesn't support switching the log name which is printed in error
>>> message depending on usage. Therefore the universal was chosen -
>>> ipaserver-ca-install.log. It was introduced by your commit
>>> d27e77adc56f5a04f3bdd1aaed5440a89ed3acad
>>>
>>> And I see, that I used wrong ticket number in the commit. Correct is
>>> https://fedorahosted.org/freeipa/ticket/6086
>>>
>>> Proper solution might be to rework main and __main__ function in
>>> ipa-ca-install but IMO we spent too much time on this already.
>>>
>>
>> Updated patch attach - it uses the other log.
>>
>>
>>
> 
> Correct me if I'm wrong but the ticket URL should be
> https://fedorahosted.org/freeipa/ticket/6086
> 

you are absolutely right. Fixed.

-- 
Petr Vobornik
From 08b2ba61e2bc390f3c8a94dc88c5d1dc0ef19288 Mon Sep 17 00:00:00 2001
From: Petr Vobornik <pvobo...@redhat.com>
Date: Fri, 15 Jul 2016 16:25:36 +0200
Subject: [PATCH] unite log file name of ipa-ca-install

ipa-ca-install said that it used
  /var/log/ipareplica-ca-install.log
but in fact it used
  /var/log/ipaserver-ca-install.log

This patch unites it to ipareplica-ca-install.log

It was chosen because of backwards compatibility - ipareplica-ca-install
was more commonly used. ipaserver-ca-install.log was used only in rare
CA less -> CA installation.

https://fedorahosted.org/freeipa/ticket/6086
---
 install/tools/ipa-ca-install | 2 +-
 ipaplatform/base/paths.py| 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/install/tools/ipa-ca-install b/install/tools/ipa-ca-install
index ed685920cbadb9cd3fc80865afb1610ca42f8b13..985e7413aa06900976934c329757ce45da5ff12d 100755
--- a/install/tools/ipa-ca-install
+++ b/install/tools/ipa-ca-install
@@ -285,7 +285,7 @@ def main():
 cainstance.is_ca_installed_locally()):
 sys.exit("CA is already installed on this host.")
 
-standard_logging_setup(paths.IPASERVER_CA_INSTALL_LOG, debug=options.debug)
+standard_logging_setup(log_file_name, debug=options.debug)
 root_logger.debug("%s was invoked with options: %s,%s",
   sys.argv[0], safe_options, filename)
 root_logger.debug("IPA version %s", version.VENDOR_VERSION)
diff --git a/ipaplatform/base/paths.py b/ipaplatform/base/paths.py
index d6fbe32f6839a5db40148777132ba1454cbc3382..1507ac36da5b40447c951ee608053a09b2db2fc3 100644
--- a/ipaplatform/base/paths.py
+++ b/ipaplatform/base/paths.py
@@ -307,7 +307,6 @@ class BasePathNamespace(object):
 IPAREPLICA_CONNCHECK_LOG = "/var/log/ipareplica-conncheck.log"
 IPAREPLICA_INSTALL_LOG = "/var/log/ipareplica-install.log"
 IPARESTORE_LOG = "/var/log/iparestore.log"
-IPASERVER_CA_INSTALL_LOG = "/var/log/ipaserver-ca-install.log"
 IPASERVER_INSTALL_LOG = "/var/log/ipaserver-install.log"
 IPASERVER_KRA_INSTALL_LOG = "/var/log/ipaserver-kra-install.log"
 IPASERVER_KRA_UNINSTALL_LOG = "/var/log/ipaserver-kra-uninstall.log"
-- 
2.5.5

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH] 963 unite log file name of ipa-ca-install

2016-07-21 Thread Petr Vobornik
On 07/19/2016 09:27 AM, Petr Vobornik wrote:
> On 07/19/2016 08:01 AM, Jan Cholasta wrote:
>> Hi,
>>
>> On 18.7.2016 18:50, Florence Blanc-Renaud wrote:
>>> On 07/15/2016 04:29 PM, Petr Vobornik wrote:
>>>> ipa-ca-install said that it used
>>>>   /var/log/ipareplica-ca-install.log
>>>> but in fact it used
>>>>   /var/log/ipaserver-ca-install.log
>>>>
>>>> This patch unites it to ipaserver-ca-install.log
>>>>
>>>> It was chosen because ipa-ca-install can be also used on
>>>> master on CA-less -> CA conversion.
>>>>
>>>> Term "server" is valid for both master and replica.
>>>>
>>>> https://fedorahosted.org/freeipa/ticket/6088
>>>>
>>>>
>>>>
>>>
>>> Looks good to me.
>>> Ack
>>
>> Does not look so good to me, "ipareplica-ca-install.log" is in fact the
>> original file name used since ipa-ca-install was introduced (in commit
>> 8a32bb3746802a29b2655e4ad2cbbba8481e1eaf), so why the switch to
>> "ipaserver-ca-install.log"?
>>
>> Honza
>>
> 
> Ideally it would be ipa-ca-install.log but for backwards compatibility,
> let's stick with one which we have. AFAIK the framework(run_script
> methodú doesn't support switching the log name which is printed in error
> message depending on usage. Therefore the universal was chosen -
> ipaserver-ca-install.log. It was introduced by your commit
> d27e77adc56f5a04f3bdd1aaed5440a89ed3acad
> 
> And I see, that I used wrong ticket number in the commit. Correct is
> https://fedorahosted.org/freeipa/ticket/6086
> 
> Proper solution might be to rework main and __main__ function in
> ipa-ca-install but IMO we spent too much time on this already.
> 

Updated patch attach - it uses the other log.

-- 
Petr Vobornik
From 4a5904b726c11c7e3323de142e5d47a5b51cff88 Mon Sep 17 00:00:00 2001
From: Petr Vobornik <pvobo...@redhat.com>
Date: Fri, 15 Jul 2016 16:25:36 +0200
Subject: [PATCH] unite log file name of ipa-ca-install

ipa-ca-install said that it used
  /var/log/ipareplica-ca-install.log
but in fact it used
  /var/log/ipaserver-ca-install.log

This patch unites it to ipareplica-ca-install.log

It was chosen because of backwards compatibility - ipareplica-ca-install
was more commonly used. ipaserver-ca-install.log was used only in rare
CA less -> CA installation.

https://fedorahosted.org/freeipa/ticket/6088
---
 install/tools/ipa-ca-install | 2 +-
 ipaplatform/base/paths.py| 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/install/tools/ipa-ca-install b/install/tools/ipa-ca-install
index ed685920cbadb9cd3fc80865afb1610ca42f8b13..985e7413aa06900976934c329757ce45da5ff12d 100755
--- a/install/tools/ipa-ca-install
+++ b/install/tools/ipa-ca-install
@@ -285,7 +285,7 @@ def main():
 cainstance.is_ca_installed_locally()):
 sys.exit("CA is already installed on this host.")
 
-standard_logging_setup(paths.IPASERVER_CA_INSTALL_LOG, debug=options.debug)
+standard_logging_setup(log_file_name, debug=options.debug)
 root_logger.debug("%s was invoked with options: %s,%s",
   sys.argv[0], safe_options, filename)
 root_logger.debug("IPA version %s", version.VENDOR_VERSION)
diff --git a/ipaplatform/base/paths.py b/ipaplatform/base/paths.py
index d6fbe32f6839a5db40148777132ba1454cbc3382..1507ac36da5b40447c951ee608053a09b2db2fc3 100644
--- a/ipaplatform/base/paths.py
+++ b/ipaplatform/base/paths.py
@@ -307,7 +307,6 @@ class BasePathNamespace(object):
 IPAREPLICA_CONNCHECK_LOG = "/var/log/ipareplica-conncheck.log"
 IPAREPLICA_INSTALL_LOG = "/var/log/ipareplica-install.log"
 IPARESTORE_LOG = "/var/log/iparestore.log"
-IPASERVER_CA_INSTALL_LOG = "/var/log/ipaserver-ca-install.log"
 IPASERVER_INSTALL_LOG = "/var/log/ipaserver-install.log"
 IPASERVER_KRA_INSTALL_LOG = "/var/log/ipaserver-kra-install.log"
 IPASERVER_KRA_UNINSTALL_LOG = "/var/log/ipaserver-kra-uninstall.log"
-- 
2.5.5

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH] 0011 server uninstall fails to remove krb principals

2016-07-19 Thread Petr Vobornik
On 07/11/2016 09:52 AM, Florence Blanc-Renaud wrote:
> Hi,
> 
> please find a patch for the 3rd issue of ticket 6012.
> 
> https://fedorahosted.org/freeipa/ticket/6012
> 
> 

bump for review

-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH] 963 unite log file name of ipa-ca-install

2016-07-19 Thread Petr Vobornik
On 07/19/2016 08:01 AM, Jan Cholasta wrote:
> Hi,
> 
> On 18.7.2016 18:50, Florence Blanc-Renaud wrote:
>> On 07/15/2016 04:29 PM, Petr Vobornik wrote:
>>> ipa-ca-install said that it used
>>>   /var/log/ipareplica-ca-install.log
>>> but in fact it used
>>>   /var/log/ipaserver-ca-install.log
>>>
>>> This patch unites it to ipaserver-ca-install.log
>>>
>>> It was chosen because ipa-ca-install can be also used on
>>> master on CA-less -> CA conversion.
>>>
>>> Term "server" is valid for both master and replica.
>>>
>>> https://fedorahosted.org/freeipa/ticket/6088
>>>
>>>
>>>
>>
>> Looks good to me.
>> Ack
> 
> Does not look so good to me, "ipareplica-ca-install.log" is in fact the
> original file name used since ipa-ca-install was introduced (in commit
> 8a32bb3746802a29b2655e4ad2cbbba8481e1eaf), so why the switch to
> "ipaserver-ca-install.log"?
> 
> Honza
> 

Ideally it would be ipa-ca-install.log but for backwards compatibility,
let's stick with one which we have. AFAIK the framework(run_script
methodú doesn't support switching the log name which is printed in error
message depending on usage. Therefore the universal was chosen -
ipaserver-ca-install.log. It was introduced by your commit
d27e77adc56f5a04f3bdd1aaed5440a89ed3acad

And I see, that I used wrong ticket number in the commit. Correct is
https://fedorahosted.org/freeipa/ticket/6086

Proper solution might be to rework main and __main__ function in
ipa-ca-install but IMO we spent too much time on this already.
-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


[Freeipa-devel] [PATCH] 963 unite log file name of ipa-ca-install

2016-07-15 Thread Petr Vobornik
ipa-ca-install said that it used
  /var/log/ipareplica-ca-install.log
but in fact it used
  /var/log/ipaserver-ca-install.log

This patch unites it to ipaserver-ca-install.log

It was chosen because ipa-ca-install can be also used on
master on CA-less -> CA conversion.

Term "server" is valid for both master and replica.

https://fedorahosted.org/freeipa/ticket/6088
-- 
Petr Vobornik
From 9af48b0d4c0b0b6d1e51cc320ec9409caa0ec873 Mon Sep 17 00:00:00 2001
From: Petr Vobornik <pvobo...@redhat.com>
Date: Fri, 15 Jul 2016 16:25:36 +0200
Subject: [PATCH] unite log file name of ipa-ca-install

ipa-ca-install said that it used
  /var/log/ipareplica-ca-install.log
but in fact it used
  /var/log/ipaserver-ca-install.log

This patch unites it to ipaserver-ca-install.log

It was chosen because ipa-ca-install can be also used on
master on CA-less -> CA conversion.

Term "server" is valid for both master and replica.

https://fedorahosted.org/freeipa/ticket/6088
---
 install/tools/ipa-ca-install | 4 ++--
 ipaplatform/base/paths.py| 1 -
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/install/tools/ipa-ca-install b/install/tools/ipa-ca-install
index ed685920cbadb9cd3fc80865afb1610ca42f8b13..eee5dee34ac375e16a17d79b0b60918ad53b2089 100755
--- a/install/tools/ipa-ca-install
+++ b/install/tools/ipa-ca-install
@@ -38,7 +38,7 @@ from ipapython.config import IPAOptionParser
 from ipapython.ipa_log_manager import root_logger, standard_logging_setup
 from ipaplatform.paths import paths
 
-log_file_name = paths.IPAREPLICA_CA_INSTALL_LOG
+log_file_name = paths.IPASERVER_CA_INSTALL_LOG
 REPLICA_INFO_TOP_DIR = None
 
 def parse_options():
@@ -285,7 +285,7 @@ def main():
 cainstance.is_ca_installed_locally()):
 sys.exit("CA is already installed on this host.")
 
-standard_logging_setup(paths.IPASERVER_CA_INSTALL_LOG, debug=options.debug)
+standard_logging_setup(log_file_name, debug=options.debug)
 root_logger.debug("%s was invoked with options: %s,%s",
   sys.argv[0], safe_options, filename)
 root_logger.debug("IPA version %s", version.VENDOR_VERSION)
diff --git a/ipaplatform/base/paths.py b/ipaplatform/base/paths.py
index d6fbe32f6839a5db40148777132ba1454cbc3382..2c221a1b48d1aa16579e9e4882f13ea6271ad1b6 100644
--- a/ipaplatform/base/paths.py
+++ b/ipaplatform/base/paths.py
@@ -303,7 +303,6 @@ class BasePathNamespace(object):
 IPABACKUP_LOG = "/var/log/ipabackup.log"
 IPACLIENT_INSTALL_LOG = "/var/log/ipaclient-install.log"
 IPACLIENT_UNINSTALL_LOG = "/var/log/ipaclient-uninstall.log"
-IPAREPLICA_CA_INSTALL_LOG = "/var/log/ipareplica-ca-install.log"
 IPAREPLICA_CONNCHECK_LOG = "/var/log/ipareplica-conncheck.log"
 IPAREPLICA_INSTALL_LOG = "/var/log/ipareplica-install.log"
 IPARESTORE_LOG = "/var/log/iparestore.log"
-- 
2.5.5

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 0186] DNS install: Ensure that DNS servers container exists

2016-07-15 Thread Petr Vobornik
On 07/15/2016 10:32 AM, Martin Babinsky wrote:
> On 07/15/2016 10:32 AM, Stanislav Laznicka wrote:
>> On 07/14/2016 05:51 PM, Martin Babinsky wrote:
>>> https://fedorahosted.org/freeipa/ticket/6083
>>>
>>>
>>>
>> ACK, works as expected.
>>
> 
> ..and putting the list back into the loop
> 

master:
* 37bfd1fdde8906b2b5712d1f99f3f4be8f91ca0a DNS install: Ensure that DNS
servers container exists

-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH] spec: require Dogtag >= 10.3.3-3

2016-07-15 Thread Petr Vobornik
On 07/12/2016 03:10 PM, Petr Spacek wrote:
> On 8.7.2016 06:52, Fraser Tweedale wrote:
>> On Thu, Jul 07, 2016 at 01:16:04PM +0200, Petr Spacek wrote:
>>> Hello,
>>>
>>> IPA 4.4.0 requires Dogtag >= 10.3.4. Is this version going to be built for
>>> Fedora any time soon?
>>>
>>> Or should I update my scripts to automatically enable
>>> COPR @freeipa/freeipa-master
>>> in my testing VMs?
>>>
>>> Thanks.
>>> Petr^2 Spacek
>>>
>> Hi Petr,
>>
>> The required features were released for Fedora as 10.3.3-3.
>> Attached patch retracts the min required version accordingly.
> 
> ACK
> 

master:
* 49389ed1e06c786df489c0fd9f6e8183f00eedff spec: require Dogtag >= 10.3.3-3

-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH 0110] schema: Fix subtopic -> topic mapping

2016-07-15 Thread Petr Vobornik
On 07/14/2016 03:09 PM, Martin Babinsky wrote:
> On 07/14/2016 01:21 PM, David Kupka wrote:
>> https://fedorahosted.org/freeipa/ticket/6069
>>
>>
> ACK.
> 

master:
* 92dea9b186611f7f1ba8aa5952b4cfdc363d75b8 schema: Fix subtopic -> topic
mapping

-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH 0014-0016][Tests] Authentication indicators

2016-07-15 Thread Petr Vobornik
On 07/14/2016 03:11 PM, Milan Kubík wrote:
> On 07/14/2016 11:43 AM, Lenka Doudova wrote:
>>

>>>
>>>
>> Resending the complete patch set.
>> L.
>>
>>
> 
> Thanks, ACK.
> 
> -- 
> Milan Kubik
> 

master:
* 0f9a5ce6b4c533647b8894f516e34bea8184f1b8 Tests: Tracker class for services
* dcdbbb975927a24ec05f7addefd59c71823a57c2 Tests: Authentication
indicators xmlrpc tests
* aab861142d3aec503ebae4779fbfa1858e20f451 Tests: Authentication
indicators integration tests

-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH 0185] messages: specify message type for ResultFormattingError

2016-07-15 Thread Petr Vobornik
On 07/14/2016 10:06 AM, Alexander Bokovoy wrote:
> On Wed, 13 Jul 2016, Martin Babinsky wrote:
>> https://fedorahosted.org/freeipa/ticket/6081
>>
>> -- 
>> Martin^3 Babinsky
> 
>> From dd2dfe4bf0a629716145af83c1b7f73595290079 Mon Sep 17 00:00:00 2001
>> From: Martin Babinsky <mbabi...@redhat.com>
>> Date: Wed, 13 Jul 2016 18:22:04 +0200
>> Subject: [PATCH] messages: specify message type for ResultFormattingError
>>
>> the ResultFormattingError message class was missing a `type` member which
>> could cause `otptoken-add` command to crash during QR image rendering
>> using
>> suboptimal TTY settings
>>
>> https://fedorahosted.org/freeipa/ticket/6081
>> ---
>> ipalib/messages.py | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/ipalib/messages.py b/ipalib/messages.py
>> index
>> 7288606f6ac923c2c87fadba5f2a6a2d9dadb7f5..6abad64a8259a8e164db60f63e75bbb9c230e7bf
>> 100644
>> --- a/ipalib/messages.py
>> +++ b/ipalib/messages.py
>> @@ -363,6 +363,7 @@ class ResultFormattingError(PublicMessage):
>> """
>> **13019** Unable to correctly format some part of the result
>> """
>> +type = "warning"
>> errno = 13019
>>
>>
> ACK.
> 

master:
* a5c8c9880d62dca50caa1cc8a77c3ae40225570b messages: specify message
type for ResultFormattingError

-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH 0179] Preserve user principal aliases during rename operation

2016-07-15 Thread Petr Vobornik
On 07/13/2016 06:07 PM, Alexander Bokovoy wrote:
> On Wed, 13 Jul 2016, Martin Babinsky wrote:
>> In that case, if nobody objects then the second revision of the patch
>> may be pushed since Alexander already acked it, right Alexander?
> Correct. ACK.

master:
* 2f02ffed03beac43b26e8521eff87b9489a746f9 Preserve user principal
aliases during rename operation

-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH 0184] vault-add: set the default vault type on the client side if none was given

2016-07-13 Thread Petr Vobornik
On 07/12/2016 03:53 PM, Stanislav Laznicka wrote:
> On 07/12/2016 02:10 PM, Martin Babinsky wrote:
>> Quick fix for https://fedorahosted.org/freeipa/ticket/6047
>>
>> Note that it depends on mbasti's patch 552 (already acked) otherwise
>> client-side vault commands would not be even visible in CLI.
>>
>>
> ACK.
> 

master:
* a1a7ecdc7bf6686adf8558cedd3964f9e4805469 vault-add: set the default
vault type on the client side if none was given

-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH] 0089 caacl: expand plugin documentation

2016-07-13 Thread Petr Vobornik
On 07/12/2016 08:45 AM, Alexander Bokovoy wrote:
> On Tue, 12 Jul 2016, Fraser Tweedale wrote:
>> Attached patch is a doc change, addressing
>> https://fedorahosted.org/freeipa/ticket/6002.
>>
>> Thanks,
>> Fraser
> 
>> From 19c5fc60391d37c9d0500feb5d5d5a6628bc4d27 Mon Sep 17 00:00:00 2001
>> From: Fraser Tweedale <ftwee...@redhat.com>
>> Date: Tue, 12 Jul 2016 15:11:11 +1000
>> Subject: [PATCH] caacl: expand plugin documentation
>>
>> Expand the 'caacl' plugin documentation to explain some common
>> confusions including the fact that CA ACLs apply to the target
>> subject principal (not necessarily the principal requesting the
>> cert), and the fact that CA-less CA ACL implies the 'ipa' CA.
>>
>> Fixes: https://fedorahosted.org/freeipa/ticket/6002
>> ---
>> ipaserver/plugins/caacl.py | 34 --
>> 1 file changed, 28 insertions(+), 6 deletions(-)
>>
>> diff --git a/ipaserver/plugins/caacl.py b/ipaserver/plugins/caacl.py
>> index
>> 9a60f7e27809c4f41b160647efafde94dbe90bf0..d316cc7c48cf2997d6be6b052dc1efa6d6fcdb6a
>> 100644
>> --- a/ipaserver/plugins/caacl.py
>> +++ b/ipaserver/plugins/caacl.py
>> @@ -23,14 +23,36 @@ if six.PY3:
>> __doc__ = _("""
>> Manage CA ACL rules.
>>
>> -This plugin is used to define rules governing which principals are
>> -permitted to have certificates issued using a given certificate
>> -profile.
>> +This plugin is used to define rules governing which CAs and profiles
>> +may be used to issue certificates to particular principals or groups
>> +of principals.
>>
>> -PROFILE ID SYNTAX:
>> +SUBJECT PRINCIPAL SCOPE:
>>
>> -A Profile ID is a string without spaces or punctuation starting with
>> a letter
>> -and followed by a sequence of letters, digits or underscore ("_").
>> +For a certificate request to be allowed, the principal(s) that are
>> +the subject of a certificate request (not necessarily the principal
>> +actually requesting the certificate) must be included in the scope
>> +of a CA ACL that also includes the target CA and profile.
>> +
>> +Users can be included by name, group or the "all users" category.
>> +Hosts can be included by name, hostgroup or the "all hosts"
>> +category.  Services can be included by service name or the "all
>> +services" category.  CA ACLs may be associated with a single type of
>> +principal, or multiple types.
>> +
>> +CERTIFICATE AUTHORITY SCOPE:
>> +
>> +A CA ACL can be associated with one or more CAs by name, or by the
>> +"all CAs" category.  For compatibility reasons, a CA ACL with no CA
>> +association implies an association with the 'ipa' CA (and only this
>> +CA).
>> +
>> +PROFILE SCOPE:
>> +
>> +A CA ACL can be associated with one or more profiles by Profile ID.
>> +The Profile ID is a string without spaces or punctuation starting
>> +with a letter and followed by a sequence of letters, digits or
>> +underscore ("_").
>>
>> EXAMPLES:
>>
> ACK. Reads well.
> 

Pushed to master: 8cd87d12d53a98a8e386c06a7c5fddb1d38d990d

-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH 0056] removed unused parameter from migrate-ds

2016-07-13 Thread Petr Vobornik
On 07/12/2016 12:35 PM, Martin Babinsky wrote:
> On 07/11/2016 12:40 PM, Stanislav Laznicka wrote:
>> https://fedorahosted.org/freeipa/ticket/6034
>>
>>
>>
> ACK
> 

master:
* 6c74bd2bcca46b586b07c3acd9670dae6e1f07b9 Removed unused method
parameter from migrate-ds


-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH 0550] host-find: do not show SSH keys by default

2016-07-13 Thread Petr Vobornik
On 07/12/2016 12:33 PM, Stanislav Laznicka wrote:
> On 07/08/2016 01:52 PM, Martin Basti wrote:
>> Reproducible only with 2+ hosts, patch attached.
>>
>> https://fedorahosted.org/freeipa/ticket/6043
>>
>>
> ACK.
> 

master:
* 2874fdbfef1e191cf858dabdd34d5a0cbdc5ef16 host-find: do not show SSH
key by default
-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH 0179] Preserve user principal aliases during rename operation

2016-07-13 Thread Petr Vobornik
p != canonical_name)
>>>>> +
>>>>> +if principals_to_add:
>>>>> +result = self.api.Command.user_add_principal(
>>>>> +obj_pkey, principals_to_add)['result']
>>>>> +
>>>>> +entry_attrs['krbprincipalname'] =
>>>>> result.get('krbprincipalname', [])
>>>>> +
>>>>> def check_mail(self, entry_attrs):
>>>>> if 'mail' in entry_attrs:
>>>>> entry_attrs['mail'] =
>>>>> self.obj.normalize_and_validate_email(entry_attrs['mail'])
>>>>> @@ -557,9 +601,11 @@ class baseuser_mod(LDAPUpdate):
>>>>>
>>>>> self.check_objectclass(ldap, dn, entry_attrs)
>>>>> self.obj.convert_usercertificate_pre(entry_attrs)
>>>>> +self.preserve_krbprincipalname_pre(ldap, entry_attrs,
>>>>> *keys,
>>>>> **options)
>>>>>
>>>>> def post_common_callback(self, ldap, dn, entry_attrs,
>>>>> *keys,
>>>>> **options):
>>>>> assert isinstance(dn, DN)
>>>>> +self.preserve_krbprincipalname_post(ldap, entry_attrs,
>>>>> **options)
>>>>> if options.get('random', False):
>>>>> try:
>>>>> entry_attrs['randompassword'] =
>>>>> unicode(getattr(context, 'randompassword'))
>>>>> --
>>>>> 2.5.5
>>>>>
>>>> The approach looks good.
>>>>
>>>> For the record, we also support aliases for hosts and service
>>>> kerberos
>>>> principals but we don't support rename options for them, so there
>>>> is no
>>>> need to add similar logic there.
>>>>
>>>>
>>> That's right, I have updated the corresponding section of the
>>> design
>>> page [1] for future reference.
>>>
>>> [1]
>>> http://www.freeipa.org/page/V4/Kerberos_principal_aliases#Managemen
>>> t_framework
>>>
>>>
>> Adding Simo to the loop since he is not convinced that this is the
>> right 
>> behavior. As I see it, it seems to not be a security issue but more
>> of a 
>> different expectations about the perceived correct behavior in this 
>> particular situation.
>>
>> I can see the use case in keeping the old aliases, e.g. keeping the
>> old 
>> credentials after legal name change, but I can also see the
>> available 
>> space for user principal names piling up and cluttering quickly in
>> large 
>> organizations.
> 
> after some thinking I think it is ok to keep by default and then drop
> as it avoid races if you do really want to keep the aliases.
> 
> However the CLI/UI should probably offer a button/switch to allow to
> drop all aliases on rename, what it would do would be to modify the
> entry after the rename and drop the aliases.
> 
> I am a bit uncertain what to do by default with the "original name".
> I can see it going both ways on whether to keep it by default or not.

Ideally we would have e.g.

 ipa user-rename oldCN --remove-aliases

which would drop everything including oldCN (I would expect that).

Unfortunately we have --rename in mod command
 ipa user-mod oldCN --rename newCn --remove-aliases

And there --remove-aliases might not be the best thing.

Do we want to support also:
  ipa user-mod CN --remove-aliases
?

> 
> Simo.
> 


-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH 0057] Don't show part of warning containing --force-ntpd in replica install

2016-07-13 Thread Petr Vobornik
On 07/13/2016 08:26 AM, Stanislav Laznicka wrote:
> On 07/12/2016 08:44 AM, Stanislav Laznicka wrote:
>> On 07/11/2016 04:27 PM, Petr Vobornik wrote:
>>> On 07/11/2016 01:23 PM, Stanislav Laznicka wrote:
>>>> https://fedorahosted.org/freeipa/ticket/6046
>>>>
>>>>
>>>>
>>> Isn't the bug about something else?
>>>
>>> The issue was that ipa-replica-install doesn't have --force-ntpd option.
>>> It is an option of ipa-client-install which is run from replica
>>> installer.
>>>
>>> The unattended mode is unrelated.
>>
>> My understanding is that the bug says that '--force-ntpd' option
>> should not be shown when ipa-client-install is run during replica
>> installation.
>>
>> During replica installation, the ipa-client-install script is run with
>> the '--unattended' flag in the 'ensure_enrolled()' function. Being a
>> separate script, there's not many options on how to pass the
>> information not to show the message to ipa-client-install. Using the
>> already used flag to get rid of the message seemed easiest to me.
>> Introducing a new 'hidden' flag (like '--from-replica'), on the other
>> hand, seems a bit harsh.
>>
> Just to throw it out there - it's possible that the '--force-join'
> client option would also appear as a hint from the client install script
> (during replica installation). Should this also be muted somehow? To me,
> it seems reasonable to rather add it as an argument to
> ipa-replica-install to pass it to the client install script.
> 

IMO client installation initiated from replica needs to have a special
option(hidden in help) similar to --on-server (or what's its name). E.g.
the name can be --replica-install. Maybe --on-server can be used but it
may have other implication which might not be valid for this use case.

Anything else are just workarounds. Imagine that admin runs
ipa-client-install with --unattended or --force-join. He would then not
get the message as now.
-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH 0552] Vault: enable client side plugins CLI

2016-07-12 Thread Petr Vobornik
On 07/12/2016 02:06 PM, Martin Babinsky wrote:
> On 07/08/2016 04:36 PM, Alexander Bokovoy wrote:
>> On Fri, 08 Jul 2016, Martin Basti wrote:
>>> Patch attached.
>>> https://fedorahosted.org/freeipa/ticket/6035
>>
>>> From 2c97c316c1db49daeda15c709f082ee083a741ad Mon Sep 17 00:00:00 2001
>>> From: Martin Basti <mba...@redhat.com>
>>> Date: Fri, 8 Jul 2016 15:53:25 +0200
>>> Subject: [PATCH] Enable vault-* commands on client
>>>
>>> Client plugins fot vault commands were disabled by NO_CLI=True,
>>> inherited from vault_add_interal, that is always NO_CLI=True.
>>> Introduced by this commit 8278da6967dbe425b4e0c6cf37dc1c53052525b2
>>>
>>> Removed NO_CLI=True from client side plugins for vault.
>>>
>>> https://fedorahosted.org/freeipa/ticket/6035
>> ACK.
>>
>> I haven't tested it but the change is obvious.
>>
> 
> And it works as expected, so ACK also from me.
> 

master:
* 9feeaca9fb552229638ce98086aa75905a45b48d Enable vault-* commands on client

-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH] kdb: check for local realm in enterprise principals

2016-07-12 Thread Petr Vobornik
On 07/11/2016 05:15 PM, Martin Babinsky wrote:
> On 07/06/2016 07:01 PM, Sumit Bose wrote:
>> Hi,
>>
>> although enterprise principals for trusted domains now are working as
>> expected they do not work for the local domain:
>>
>> # kinit -E admin@IPA.DEVEL
>> kinit: Client 'admin\@IPA.DEVEL@IPA.DEVEL' not found in Kerberos
>> database while getting initial credentials
>>
>> Attached patch handles this case. It is not that nice because of the
>> duplication of ipadb_fetch_principals() and ipadb_find_principal(). But
>> I think there was a reason I do not remember why we didn't check for
>> enterprise principals before checking the local database. If there is no
>> such reason it might make sense to check for enterprise principals
>> before doing the lookup. Please let me know if I should change the patch
>> accordingly or if the current version is ok,
>>
>> bye,
>> Sumit
>>
>>
>>
> Code looks ok to me and the patch fixes the issue, so ACK.
> 

master:
* 6d6da6b281173737bd31ba4845af11a097846c05 kdb: check for local realm in
enterprise principals

-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH 0183] ipa-advise: correct handling of plugin namespace iteration

2016-07-12 Thread Petr Vobornik
On 07/11/2016 02:30 PM, Stanislav Laznicka wrote:
> On 07/11/2016 02:18 PM, Martin Babinsky wrote:
>> https://fedorahosted.org/freeipa/ticket/6044
>>
>>
>>
> ACK.
> 
> 
> 
master:
* c1d8629b7490f443eededf0c0d0472d8285f85e8 ipa-advise: correct handling
of plugin namespace iteration

-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH 0182] ipa-compat-manage: use server API to retrieve plugin statu

2016-07-12 Thread Petr Vobornik
On 07/12/2016 10:22 AM, Stanislav Laznicka wrote:
> On 07/12/2016 10:02 AM, Stanislav Laznicka wrote:
>> On 07/11/2016 10:50 AM, Martin Babinsky wrote:
>>> Fixes regression reported in
>>> https://fedorahosted.org/freeipa/ticket/6033
>>>
>> Hello,
>>
>> The ticket is rather cryptic as it has ipa-compat-manage in header but
>> describes error in ipa-nis-manage. Both scripts suffer with the very
>> same error, could you fix the latter as well?
>>
>> Thanks,
>> Standa
> 
> Never mind, found the ipa-nis-manage patch posted earlier in the mailing
> list ACKed although it probably hasn't been pushed yet.
> 
> ACK.
> 

master:
* a5efeb449bba47dd430a7b8ffa594ace189252f4 ipa-compat-manage: use server
API to retrieve plugin status

-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [patch 0038-0040] Sub CA test patches

2016-07-12 Thread Petr Vobornik
On 07/08/2016 06:06 AM, Fraser Tweedale wrote:
> On Thu, Jul 07, 2016 at 03:46:52PM +0200, Milan Kubík wrote:
>> On 07/04/2016 08:57 AM, Fraser Tweedale wrote:
>>> Hi Milan,

>>
> Thanks Milan,
> 
> All working for me now.  ACK on all four patches.
> 
> Cheers,
> Fraser
> 

master:
* ea9b15f435c6327c6f642e3e8093796229d94598 ipatests: Tracker
implementation for Sub CA feature
* 5b37aaad7718bd0214053fd2e758ba7dc332e21d ipatests: Extend CAACL suite
to cover Sub CA members
* d88a12f1f59640bb6593169aa4c7ea204af18cee ipatests: Test Sub CA with
CAACL and certificate profile
* 0277a89825cf0d8d1099f537d9eb4ab1020751d2 ipatests: remove ipacertbase
option from test CSR configuration


-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH 0181] ipa-nis-manage: Use server API to retrieve plugin status

2016-07-12 Thread Petr Vobornik
On 07/06/2016 11:18 AM, Florence Blanc-Renaud wrote:
> On 07/04/2016 01:36 PM, Martin Babinsky wrote:
>> https://fedorahosted.org/freeipa/ticket/6027
>>
>>
>>
> Hi Martin,
> 
> I tested your patch and it correctly fixes the issue.
> Ack,
> 
> Flo.
> 

master:
* c5cc79f1ad2ef1eb81ad3d9cea2882a7ae1825b2 ipa-nis-manage: Use server
API to retrieve plugin status

-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH] 0087 uninstall: untrack lightweight CA certs

2016-07-12 Thread Petr Vobornik
On 07/04/2016 10:18 AM, Martin Babinsky wrote:
> On 07/04/2016 05:10 AM, Fraser Tweedale wrote:
>> The attached patch fixes
>> https://fedorahosted.org/freeipa/ticket/6020
>>
>> Thanks,
>> Fraser
>>
>>
>>
> ACK.
> 

master:
* 88841a561922fd9a57f3c473833f2ff26c8061ec uninstall: untrack
lightweight CA certs

-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH 0057] Don't show part of warning containing --force-ntpd in replica install

2016-07-11 Thread Petr Vobornik
On 07/11/2016 01:23 PM, Stanislav Laznicka wrote:
> https://fedorahosted.org/freeipa/ticket/6046
> 
> 
> 
Isn't the bug about something else?

The issue was that ipa-replica-install doesn't have --force-ntpd option.
It is an option of ipa-client-install which is run from replica installer.

The unattended mode is unrelated.
-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] Proposed patch to resolve #828866 [RFE] enhance --subject option for ipa-server-install

2016-07-07 Thread Petr Vobornik

On 07/07/2016 03:16 PM, Rob Crittenden wrote:

Sebastian Hetze wrote:

Hi *

attached you find a patch that adds new options --subject_cn and
--subject_mail to ipa-server-install that make the CA cert subject CN
customizable.

This patch has been tested by a customer in a PoC.
However, i assume additional testing in different environments is
required.

It would be greatly appreciated if this patch would find its way into
the product very soon.


I don't see the advantage of passing around the subject_rdn along with
the base. Why not pre-combine them into a DN?

Similarly, I think I'd drop storing the subject base and RDN and just
store just the DN. I don't think there would be any backwards compat
issues as this would only apply to new installs.

I think this would explode the number of options as users request
additional attributes for the subject (OU, C, etc). Might be better to
make the user pass in a full DN if they want to manage the CA subject. I
don't know if any validation would be required for dogtag (e.g. is there
a minimum set of components needed?)


+1, IMO using e.g., --subject="e=mail,cn=Something,O=REALM.NAME" is 
better. But IPA should validate it properly to disallow anything not 
usable by dogtag.


Adding Fraser for the dogtag part.



rob



--
Petr Vobornik

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH] kdb: check for local realm in enterprise principals

2016-07-07 Thread Petr Vobornik

On 07/06/2016 07:01 PM, Sumit Bose wrote:

Hi,

although enterprise principals for trusted domains now are working as
expected they do not work for the local domain:

# kinit -E admin@IPA.DEVEL
kinit: Client 'admin\@IPA.DEVEL@IPA.DEVEL' not found in Kerberos database 
while getting initial credentials

Attached patch handles this case. It is not that nice because of the
duplication of ipadb_fetch_principals() and ipadb_find_principal(). But
I think there was a reason I do not remember why we didn't check for
enterprise principals before checking the local database. If there is no
such reason it might make sense to check for enterprise principals
before doing the lookup. Please let me know if I should change the patch
accordingly or if the current version is ok,

bye,
Sumit



Hi Sumit,

thanks for the patch. This patch should have a ticket. It will help 
downstream planning.


--
Petr Vobornik

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH] 0010 Show full error message for selinuxusermap-add-hostgroup

2016-07-07 Thread Petr Vobornik

On 07/05/2016 02:38 PM, Florence Blanc-Renaud wrote:

Hi,

the output of ipa selinuxusermap-add-hostgroup and
selinuxusermap-add-user does not display any more the host/host group or
user/group that could not be added. This patch fixes this regression by
adding the labels host/hostgroup/user/group to the list of
_failed_member_output_params of the class ClientMethod.


https://fedorahosted.org/freeipa/ticket/6026



I've a feeling that this issue is more general and multiple commands 
regressed. Would be good to check other member options, e.g. also in 
user plugin.


--
Petr Vobornik

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


[Freeipa-devel] FreeIPA 4.4.0 tagged

2016-07-01 Thread Petr Vobornik
FreeIPA 4.4.0 was tagged.

Release notes will follow soon.

* http://www.freeipa.org/page/Downloads#Latest_Release_-_FreeIPA_4.4.0
* http://freeipa.org/downloads/src/freeipa-4.4.0.tar.gz
SHA1: 441ef8cb2b0ac103723d03b0478da641d697e104
MD5: 078697b25e02361fca37d00a1144130d
-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH] pwpolicy: Do not expire passwords when maxlife is set to 0 (infinity).

2016-07-01 Thread Petr Vobornik
On 07/01/2016 10:46 AM, David Kupka wrote:
> Hello Thierry!
> 
> Thanks for looking into it. I will try to answer your questions and
> comments inline.
> 
> On 01/07/16 10:26, thierry bordaz wrote:
>> Hi David,
>>
>> The patch looks good but being not familiar with that code, my comments
>> may be absolutely wrong
>>
>> In ipadb_get_pwd_expiration, if it is not 'self' we set
>> '*export=mod_time'.
>> If for some reason 'mod_time==0', it has now a specific meaning 'not
>> expiring' . Does it match the comment '* not 'self', so reset */'
> 
> mod_time should be timestamp of the modification operation. So mod_time
> == 0 should happen only when the change was performed in the very
> beginning of 70's.
> 
>>
>> In ipadb_entry_to_mods, it deletes krbPasswordExpiration. But just
>> before it adds in the mods krbPasswordExpiration=0 or
>> krbPasswordExpiration=entry->pw_expiration
>> Could we skip those mods if entry->pw_expiration==0 or expire_time==0 ?
> 
> That is exactly what I thought. But in that part of code I have no
> chance to check if the attribute is present in the entry or not. Also I
> can't catch and ignore the resulting error when deleting nonexistent
> attribute. Here only LDAPMod structures are filled but the execution is
> done in an other part of code.
> So I choose the easy path and always set the attribute and delete it
> right after if necessary.
> 
>>
>> In ipapwd_SetPassword, ipapwd_post_modadd, same remark as above.
>>
>> Something that I am not sure is what is the expected relation between
>> passwordexpirationtime and krbPasswordExpiration
> 
> I'm not sure either. I think we don't use passwordexpirationtime attribute.
> 
>>
>> thanks
>> thierry

I don't see a strict NACK here. Given pvomacka's functional ACK, I have
pushed it. We can fix corner cases later.

Pushed to master: d2cb9ed327ee4003598d5e45d80ab7918b89eeed

>>
>> On 06/30/2016 09:34 PM, David Kupka wrote:
>>> On 04/05/16 17:22, Pavel Vomacka wrote:
>>>>
>>>>
>>>> On 05/04/2016 04:36 PM, Simo Sorce wrote:
>>>>> On Wed, 2016-05-04 at 15:39 +0200, Martin Kosek wrote:
>>>>>> On 05/02/2016 02:28 PM, David Kupka wrote:
>>>>>>> https://fedorahosted.org/freeipa/ticket/2795
>>>>>> That patch looks suspiciously short given the struggles I saw in
>>>>>> http://www.redhat.com/archives/freeipa-devel/2015-June/msg00198.html
>>>>>> :-)
>>>>>>
>>>>>> Instead of setting to IPAPWD_END_OF_TIME, should we instead avoid
>>>>>> filling
>>>>>> "krbPasswordExpiration" attribute at all, i.e. have password
>>>>>> *without*
>>>>>> expiration? Or is krbPasswordExpiration mandatory?
>>>>> So I looked at the MIT code, and it seem like they are coping just
>>>>> fine
>>>>> with a missing (ie value = 0 internally) pw_expiration attribute.
>>>>>
>>>>> So if we make our code cope with omitting any expiration if the
>>>>> attribute is missing then yes, we can mark no expiration with simply
>>>>> removing (or not setting) the krbPasswordExpiration attribute.
>>>>> The attribute itself is optional and can be omitted.
>>>>>
>>>>> I think this is a good idea, and is definitely better than inventing
>>>>> a a
>>>>> magic value.
>>>>>
>>>>> Simo.
>>>>>
>>>> Just a note: I tested David's patch and it actually doesn't work when
>>>> the new password policy for ipausers group is created (priority = 0,
>>>> which should be the highest priority). The maxlife and minlife values
>>>> are empty. Even if I set the new password policy maxlife and minlife to
>>>> 0 the result was that password will expire in 90 days. The patch worked
>>>> correctly when I changed value of maxlife and minlife to 0 in
>>>> 'global_policy'. Then the password expiration was set to 2038-01-01.
>>>>
>>>
>>> Hello!
>>>
>>> I hope I've finally find all the places in ipa-kdb and ipa-pwd-extop
>>> plugins to tickle in order to have password that don't expire. Updated
>>> patch attached.
>>>
>>> https://fedorahosted.org/freeipa/ticket/2795
>>>
>>
-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH] 0085 Fix upgrade when Dogtag also upgraded from 10.2 -> 10.3

2016-07-01 Thread Petr Vobornik
On 07/01/2016 11:02 AM, Martin Babinsky wrote:
> On 06/30/2016 01:16 PM, Fraser Tweedale wrote:
>> Hullo,
>>
>> The attached patch fixes
>> https://fedorahosted.org/freeipa/ticket/6011.
>>
>> Cheers,
>> Fraser
>>
>>
>>
> ACK
> 

Pushed to master: 3691e39a62da5134f911f6a798f79a3a2ae0c025

-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH 558] Allow disabling requireing preauth by default for Service Principal Names

2016-07-01 Thread Petr Vobornik
On 03/08/2016 06:02 PM, Martin Babinsky wrote:
> On 03/08/2016 05:50 PM, Simo Sorce wrote:
>> On Tue, 2016-03-08 at 17:20 +0100, Martin Babinsky wrote:
>>> On 03/08/2016 05:00 PM, Simo Sorce wrote:
>>>> On Tue, 2016-03-08 at 16:51 +0100, Martin Babinsky wrote:
>>>>> On 03/08/2016 04:49 PM, Simo Sorce wrote:
>>>>>> On Fri, 2015-12-04 at 14:23 +0100, Martin Babinsky wrote:
>>>>>>> On 12/01/2015 10:08 PM, Simo Sorce wrote:
>>>>>>>> On Tue, 2015-12-01 at 15:59 +0100, Martin Babinsky wrote:
>>>>>>>>> On 11/30/2015 07:42 PM, Simo Sorce wrote:
>>>>>>>>>> On Wed, 2015-11-25 at 10:33 +0100, Martin Babinsky wrote:
>>>>>>>>>>> On 11/24/2015 10:20 PM, Simo Sorce wrote:
>>>>>>>>>>>> This addresses #3860, giving admins the option to not
>>>>>>>>>>>> require preauth
>>>>>>>>>>>> for Hosts and services.
>>>>>>>>>>>>
>>>>>>>>>>>> I did not add this option by default, although it does
>>>>>>>>>>>> reduce the load
>>>>>>>>>>>> on the KDC as well as speed up TGT acquisition for service
>>>>>>>>>>>> principal
>>>>>>>>>>>> accounts that acquire TGTs.
>>>>>>>>>>>>
>>>>>>>>>>>> Tested and working as expected (SPNs are not returned
>>>>>>>>>>>> PREAUTH_NEEDED
>>>>>>>>>>>> error while normal users are).
>>>>>>>>>>>>
>>>>>>>>>>>> HTH,
>>>>>>>>>>>> Simo.
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>> Hi Simo,
>>>>>>>>>>>
>>>>>>>>>>> I was not able to apply the patch on current master branch:
>>>>>>>>>>>
>>>>>>>>>>> """
>>>>>>>>>>> git am
>>>>>>>>>>> ../review/ssorce/3860/freeipa-simo-558-1-Allow-admins-to-disable-preauth-for-SPNs.patch
>>>>>>>>>>>
>>>>>>>>>>> -3
>>>>>>>>>>>
>>>>>>>>>>> Applying: Allow admins to disable preauth for SPNs.
>>>>>>>>>>> error: invalid object 100644
>>>>>>>>>>> a6b4d4349a9ac6de453d9ad3c679ec32add4e43b
>>>>>>>>>>> for 'ipalib/plugins/config.py'
>>>>>>>>>>> fatal: git-write-tree: error building trees
>>>>>>>>>>> Repository lacks necessary blobs to fall back on 3-way merge.
>>>>>>>>>>> Cannot fall back to three-way merge.
>>>>>>>>>>> Patch failed at 0001 Allow admins to disable preauth for SPNs.
>>>>>>>>>>> """
>>>>>>>>>>>
>>>>>>>>>>> It seems that I nedd to apply some of your other patches
>>>>>>>>>>> first (which one?)
>>>>>>>>>>
>>>>>>>>>> Sorry did not see this question earlier, it requires 556 and
>>>>>>>>>> 557, I just
>>>>>>>>>> bumped that thread.
>>>>>>>>>>
>>>>>>>>>> Simo.
>>>>>>>>>>
>>>>>>>>> It seems that I need something else, patch 556-2 applies
>>>>>>>>> cleanly, but
>>>>>>>>> patch 557-3 fails with http://fpaste.org/296230/89819431/ on
>>>>>>>>> both master
>>>>>>>>> and 4-2 branch.
>>>>>>>>>
>>>>>>>>
>>>>>>>> Rebased 556,557 in their thread, and here is the rebase for 558
>>>>>>>> on top
>>>>>>>> of them.
>>>>>>>>
>>>>>>>> Simo.
>>>>>>>>
>>>>>>>
>>>>>>> ACK. I'm afraid that this patch and 556, 557 will require another
>>>>>>> round
>>>>>>> of rebase before pushing, though.
>>>>>>
>>>>>> Rebased on top of master (not on 556/557) per Petr's request.
>>>>>>
>>>>>> Simo.
>>>>>>
>>>>>>
>>>>>
>>>>> NACK, if you do API changes please increment API version in VERSION.
>>>>
>>>> Why wasn't this a problem in the previous ACK ?
>>>>
>>>> Simo.
>>>>
>>>
>>> Probably because I missed it, sorry.
>>>
>>
>> Fixed.
>>
>> Simo.
>>
> 
> Thanks, ACK.
> 

was this pushed?

-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH] 0067-72: webui for kerberos aliases

2016-07-01 Thread Petr Vobornik
On 07/01/2016 09:04 AM, Pavel Vomacka wrote:
> 
> 
> On 06/30/2016 05:27 PM, Petr Vobornik wrote:
>> On 06/30/2016 02:48 PM, Pavel Vomacka wrote:
>>> Hello,
>>>
>>> please review these patches. First two patches fix two minor bugs in
>>> custom_command_multivalued_widget.
>>>
>>> The rest of patches add webui for kerberos aliases.
>>>
>>> https://fedorahosted.org/freeipa/ticket/5927
>>>
>> A preliminary review - I only read the code.
>>
>> Patch 0067: LGTM,
>>
>> btw same wrong interface is in on_error_add, but there it is not use
> fixed for on_error_add as well.
>>
>> Patch 0068: lGTM
>>
>> Patch 0069:
>>
>> 1. A nitpick, not necessarily a NACK.
>> krb_custom_command_multivalued_widget should be named e.g.
>> krb_principal_multivalued_widget.
>>
>> 2. Doc texts for the new widges are missing. This can be added later.
>>
>>
>> 3. `!principal_name || principal_name === '')` is the same as
>> `!principal_name`
>>
>> so
>>
>> var principal_name = value[0];
>>
>>  if (!principal_name || principal_name === '') {
>>  principal_name = {};
>>  }
>>
>> could be simplified into
>>var principal_name = value[0] || {};
>>
>> but why is an object set into that.principal_name when it is later used
>> as a text: `that.principal_text.text(that.principal_name);`
>>
> fixed
>> Patch 0070: LGTM
>>
>> Patch 0071: LGTM
>>
>> Patch 0072: LGTM if the change of krbprincipalname to krbcanonicalname
>> is good.
>>
>>
> Updated patches attached.
> 

Focusing on element was returned to patch 68 as discussed offline.

ACK for all.

master:
* df56fd3371bd20a2ce8f5d0097e05437b7827e29 Change error handling in
custom_command_multivalued_widget
* 2232a5bb09b3e99d10598ab64d0bf5d8ef006df4 Set default confirmation
button label to 'Remove'
* 4bc2e3164fbc4fdbbd4ecd1d26001a5d4671dd94 Add widgets for kerberos aliases
* 2da3090a9716bc47e9cf29329ac9bdb734376cb6 Add widget for kerberos
aliases to user page
* 62c4e15d16cf1b29d4a23db146c774ba01bf5935 Add widget for kerberos
aliases to hosts page
* 2ec59b7f232d9119d32d7a5574efba8965904ee8 Add widget for kerberos
aliases to service page

-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


[Freeipa-devel] [PATCH] 961 webui: prevent infinite reload for users with krbbprincipal alias set

2016-06-30 Thread Petr Vobornik
Web UI has an inbuilt mechanism to reload in case response from a server
contains a different principal than the one loaded during Web UI
startup.

see rpc.js:381

With kerberos aliases support the loaded principal could be different
because krbprincipalname contained multiple values.

In such case krbcanonicalname should be used - it contains the same
principal as the one which will be in future API responses.

part of: https://fedorahosted.org/freeipa/ticket/5927
-- 
Petr Vobornik
From a15518f25eb339ab2bc90dbc304648a9fd266e51 Mon Sep 17 00:00:00 2001
From: Petr Vobornik <pvobo...@redhat.com>
Date: Thu, 30 Jun 2016 19:16:32 +0200
Subject: [PATCH] webui: prevent infinite reload for users with krbbprincipal
 alias set

Web UI has inbuilt mechanism to reload in case response from a server
contains a different principal than the one loaded during Web UI
startup.

see rpc.js:381

With kerberos aliases support the loaded principal could be different
because krbprincipalname contained multiple values.

In such case krbcanonicalname should be used - it contains the same
principal as the one which will be in future API responses.

https://fedorahosted.org/freeipa/ticket/5927
---
 install/ui/src/freeipa/ipa.js | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/install/ui/src/freeipa/ipa.js b/install/ui/src/freeipa/ipa.js
index 830def0542faeb14b62b71fb80c753bc121cace7..e8ad832c3aacc0fcd824af3e1956ff621ae761ed 100644
--- a/install/ui/src/freeipa/ipa.js
+++ b/install/ui/src/freeipa/ipa.js
@@ -259,7 +259,11 @@ var IPA = function () {
 },
 on_success: function(data, text_status, xhr) {
 that.whoami = batch ? data.result[0] : data.result.result[0];
-that.principal = that.whoami.krbprincipalname[0];
+var cn = that.whoami.krbcanonicalname;
+if (cn) that.principal = cn[0];
+if (!that.principal) {
+that.principal = that.whoami.krbprincipalname[0];
+}
 }
 });
 };
-- 
2.5.5

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH] 0067-72: webui for kerberos aliases

2016-06-30 Thread Petr Vobornik
On 06/30/2016 02:48 PM, Pavel Vomacka wrote:
> Hello,
> 
> please review these patches. First two patches fix two minor bugs in
> custom_command_multivalued_widget.
> 
> The rest of patches add webui for kerberos aliases.
> 
> https://fedorahosted.org/freeipa/ticket/5927
> 

A preliminary review - I only read the code.

Patch 0067: LGTM,

btw same wrong interface is in on_error_add, but there it is not use


Patch 0068: lGTM

Patch 0069:

1. A nitpick, not necessarily a NACK.
krb_custom_command_multivalued_widget should be named e.g.
krb_principal_multivalued_widget.

2. Doc texts for the new widges are missing. This can be added later.


3. `!principal_name || principal_name === '')` is the same as
`!principal_name`

so

var principal_name = value[0];

if (!principal_name || principal_name === '') {
principal_name = {};
}

could be simplified into
  var principal_name = value[0] || {};

but why is an object set into that.principal_name when it is later used
as a text: `that.principal_text.text(that.principal_name);`


Patch 0070: LGTM

Patch 0071: LGTM

Patch 0072: LGTM if the change of krbprincipalname to krbcanonicalname
is good.


-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH 0096] Add authentication indicators support to Host objects

2016-06-30 Thread Petr Vobornik
On 06/30/2016 03:55 PM, Nathaniel McCallum wrote:
> On Thu, 2016-06-30 at 13:42 +0200, Petr Vobornik wrote:
>> On 06/29/2016 04:40 PM, Stanislav Laznicka wrote:
>>>
>>> On 06/29/2016 04:02 PM, Stanislav Laznicka wrote:
>>>>
>>>> On 06/29/2016 03:53 PM, Martin Basti wrote:
>>>>>
>>>>>
>>>>>
>>>>> On 29.06.2016 15:52, Stanislav Laznicka wrote:
>>>>>>
>>>>>> On 06/24/2016 03:14 PM, Martin Basti wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On 24.06.2016 15:11, Sumit Bose wrote:
>>>>>>>>
>>>>>>>> On Tue, Jun 21, 2016 at 02:25:49PM -0400, Nathaniel
>>>>>>>> McCallum wrote:
>>>>>>>>>
>>>>>>>>> https://fedorahosted.org/freeipa/ticket/433
>>>>>>>> The patch works for me as expected, but the API.txt
>>>>>>>> update is
>>>>>>>> missing in
>>>>>>>> the patch.
>>>>>>>>
>>>>>>>> bye,
>>>>>>>> Sumit
>>>>>>>
>>>>>>> There are no updated managed permissions for
>>>>>>> krbprincipalauthind
>>>>>>> attribute in hosts.py, is this omitted on purpose?
>>>>>>> Martin^2
>>>>>>>
>>>>>> The attached patch adds them should these be required.
>>>>>>
>>>>>>
>>>>>
>>>>> Then we also needs patch for services.py, because there are
>>>>> missing
>>>>> ACIs too
>>>>>
>>>>> Martin^2
>>>>
>>>> It was already included but let me separate it in two patches,
>>>> then.
>>>>
>>>>
>>> Good catch from Petr Vobornik - the rebuilt ACI.txt should also be
>>> included.
>>>
>>
>> Attaching new version of Nathnaniel's patch with API.txt and VERSION
>> updated.
>>
>> ACK for 0096-2
>>
>> Pushed to master
>> * 0855b014b1edcb1632a41e380220abd7bb5e481a Add authentication
>> indicators
>> support to Host objects.
>>
>> The  "{Service|Host} {Read|Modify} " permissions looks good to me.
>> ACK
>> if Nathaniel agrees that it doesn't deserved it's own permission for
>> modify.
> 
> I agree. We can add it later if someone needs it.
> 

pushed to master:
* 97db87b383b1ae4639bdb51793354bad30adf5a9 host: Added permissions for
auth. indicators read/modify
* 235b19ba7f9807ecf10436d1a5b28518b4475a70 service: Added permissions
for auth. indicators read/modify


-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH] 0007 Fix ipa-server-certinstall with certs signed by 3rd-party CA

2016-06-30 Thread Petr Vobornik
On 06/29/2016 02:17 PM, Stanislav Laznicka wrote:
> On 06/22/2016 09:29 PM, Florence Blanc-Renaud wrote:
>> Hi,
>>
>> This patch fixes ipa-server-certinstall when used with 3rd-party certs.
>> The scenario is the following:
>> - install the server with an embedded CA
>> - use ipa-cacert-manage to install a 3rd party CA
>> - use ipa-certupdate to put the 3rd party CA cert in the relevant NSS 
>> databases (/etc/ipa/nssdb /etc/httpd/alias /etc/pki/pki-tomcat/alias and 
>> /etc/dirsrv/slapd-XXX)
>> - use ipa-server-certinstall to replace the Directory/Apache server 
>> certificates with a cert signed by the 3rd party CA.
>>
>> Note that I had to run ipa-certupdate after putting selinux mode to 
>> permissive 
>> (otherwise the cert does not get into /etc/pki/pki-tomcat/alias) and a bz 
>> has 
>> been opened against selinux-policy to solve this issue.
>>
>> https://fedorahosted.org/freeipa/ticket/4785
>> https://fedorahosted.org/freeipa/ticket/4786
>>
>>
> Hello,
> 
> The patch works as expected with the selinux requirement you mentioned. I 
> will 
> just add Honza for code sanity check. Therefore conditional ACK if the code 
> can 
> take no further improvements.
> 
> Standa
> 
> 

Pushed to master: 025cfd911bce6214ef2b4311b16c5b6df6ad173a

According to Honza, it doesn't solve all corner cases. This can be fixed
in a future. Honza, please open a ticket with the corner cases when you
have time.

-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH] 0062, 63: webui: Add button for 'server-del' command

2016-06-30 Thread Petr Vobornik
On 06/30/2016 01:57 PM, Pavel Vomacka wrote:
> 
> 
> On 06/29/2016 05:42 PM, Petr Vobornik wrote:
>> On 06/24/2016 12:40 PM, Pavel Vomacka wrote:
>>> Hello,
>>>
>>> please review attached patches, they add 'Delete Server' button.
>>>
>> 1. there is a whitespace warning while applying patch 63.
>>
>> 2. It breaks expectation of no_init.
>> Instead of
>>var that = IPA.details_facet(spec);
>> Use
>>var that = IPA.details_facet(spec, no_init);
>>
>> Then rename:
>>that.init_facet = function() {
>> ẗo
>>that.init_servers_facet = function() {
>>
>> Add there at beginning:
>>that.init_details_facet
>>
>>
>> 3. IPA.action and IPA.delete_action has the functionality of
>> server_delete_action build-in, including redirection to server search
>> page which is missing in this patch.
>>
>> Updated patch with the issues fixed is attached.
>>
>> I did not test final version of the patch because my testing env. died.
> Thank you, everything works well.
> 
> ACK
> 

master:
* e65ce4fedc8e8e4f16c8d0c56f7618a84e8e7f85 Add support to change button
css class on confirm dialog
* 7f4de88ea1da11d4111a917672c0cb7ab9866c90 Add button for server-del command

-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH] 0064: webui: simplify confirmation messages in confirmation dialogs

2016-06-30 Thread Petr Vobornik
On 06/30/2016 10:39 AM, Pavel Vomacka wrote:
> 
> 
> On 06/29/2016 04:40 PM, Petr Vobornik wrote:
>> On 06/27/2016 05:50 PM, Pavel Vomacka wrote:
>>> Hello,
>>>
>>> Please review attached patch which simplifies confirmation messages for
>>> 'remove cert hold' and 'restore cert' actions.
>>>
>> I'd change:
>>You can select a reason from the pull-down list.
>> To:
>>Select a reason from the pull-down list.
>>
> Changed. Updated patch attached.
> 

ACK

Pushed to master: a3c7f845e019f79c2cefde0526790dec4f118b56

-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH] 0065, 66: webui: authentication indicators on host page

2016-06-30 Thread Petr Vobornik
On 06/29/2016 06:38 PM, Petr Vobornik wrote:
> On 06/28/2016 04:32 PM, Pavel Vomacka wrote:
>> Hello,
>>
>> please review attached patches. I moved strings used by authentication
>> indicators widget to another dict so the second patch changes strings in
>> custom_checkbox widget on service page.
>>
>> https://fedorahosted.org/freeipa/ticket/5872
>>
> 
> 
> ACK
> 
> push should wait on server side.
> 

master:
* 55049fceb978f2e20b13800b5377428de386 Add authentication
identificator to host page
* ec6925e775598602e909d7a1f226f0c1e28cb074 Change paths of strings in
auth indicators widget on service page
-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH 0096] Add authentication indicators support to Host objects

2016-06-30 Thread Petr Vobornik
On 06/29/2016 04:40 PM, Stanislav Laznicka wrote:
> On 06/29/2016 04:02 PM, Stanislav Laznicka wrote:
>> On 06/29/2016 03:53 PM, Martin Basti wrote:
>>>
>>>
>>> On 29.06.2016 15:52, Stanislav Laznicka wrote:
>>>> On 06/24/2016 03:14 PM, Martin Basti wrote:
>>>>>
>>>>>
>>>>> On 24.06.2016 15:11, Sumit Bose wrote:
>>>>>> On Tue, Jun 21, 2016 at 02:25:49PM -0400, Nathaniel McCallum wrote:
>>>>>>> https://fedorahosted.org/freeipa/ticket/433
>>>>>> The patch works for me as expected, but the API.txt update is
>>>>>> missing in
>>>>>> the patch.
>>>>>>
>>>>>> bye,
>>>>>> Sumit
>>>>>
>>>>> There are no updated managed permissions for krbprincipalauthind
>>>>> attribute in hosts.py, is this omitted on purpose?
>>>>> Martin^2
>>>>>
>>>> The attached patch adds them should these be required.
>>>>
>>>>
>>>
>>> Then we also needs patch for services.py, because there are missing
>>> ACIs too
>>>
>>> Martin^2
>>
>> It was already included but let me separate it in two patches, then.
>>
>>
> Good catch from Petr Vobornik - the rebuilt ACI.txt should also be
> included.
> 

Attaching new version of Nathnaniel's patch with API.txt and VERSION
updated.

ACK for 0096-2

Pushed to master
* 0855b014b1edcb1632a41e380220abd7bb5e481a Add authentication indicators
support to Host objects.

The  "{Service|Host} {Read|Modify} " permissions looks good to me. ACK
if Nathaniel agrees that it doesn't deserved it's own permission for
modify.
-- 
Petr Vobornik
From 3de08f354a8714a752b567850968b57ffc44553d Mon Sep 17 00:00:00 2001
From: Nathaniel McCallum <npmccal...@redhat.com>
Date: Tue, 21 Jun 2016 14:19:03 -0400
Subject: [PATCH] Add authentication indicators support to Host objects

https://fedorahosted.org/freeipa/ticket/433
---
 API.txt   |  9 ++---
 VERSION   |  4 ++--
 ipaserver/plugins/host.py | 17 -
 3 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/API.txt b/API.txt
index 76e58aeec4301577952f919b17a58b71c06a..19922660ad1787d87337b37e099c7fd9475eda53 100644
--- a/API.txt
+++ b/API.txt
@@ -2257,7 +2257,7 @@ output: Output('summary', type=[, ])
 output: Output('value', type=[])
 output: Output('warning', type=[, , ])
 command: host_add/1
-args: 1,23,3
+args: 1,24,3
 arg: Str('fqdn', cli_name='hostname')
 option: Str('addattr*', cli_name='addattr')
 option: Flag('all', autofill=True, cli_name='all', default=False)
@@ -2268,6 +2268,7 @@ option: Str('ipaassignedidview?')
 option: Bool('ipakrbokasdelegate?', cli_name='ok_as_delegate')
 option: Bool('ipakrbrequirespreauth?', cli_name='requires_pre_auth')
 option: Str('ipasshpubkey*', cli_name='sshpubkey')
+option: Str('krbprincipalauthind*', cli_name='auth_ind')
 option: Str('l?', cli_name='locality')
 option: Str('macaddress*')
 option: Flag('no_members', autofill=True, default=False)
@@ -2380,7 +2381,7 @@ output: Output('completed', type=[])
 output: Output('failed', type=[])
 output: Entry('result')
 command: host_find/1
-args: 1,34,4
+args: 1,35,4
 arg: Str('criteria?')
 option: Flag('all', autofill=True, cli_name='all', default=False)
 option: Str('description?', autofill=False, cli_name='desc')
@@ -2392,6 +2393,7 @@ option: Str('in_netgroup*', cli_name='in_netgroups')
 option: Str('in_role*', cli_name='in_roles')
 option: Str('in_sudorule*', cli_name='in_sudorules')
 option: Str('ipaassignedidview?', autofill=False)
+option: Str('krbprincipalauthind*', autofill=False, cli_name='auth_ind')
 option: Str('l?', autofill=False, cli_name='locality')
 option: Str('macaddress*', autofill=False)
 option: Str('man_by_host*', cli_name='man_by_hosts')
@@ -2421,7 +2423,7 @@ output: ListOfEntries('result')
 output: Output('summary', type=[, ])
 output: Output('truncated', type=[])
 command: host_mod/1
-args: 1,24,3
+args: 1,25,3
 arg: Str('fqdn', cli_name='hostname')
 option: Str('addattr*', cli_name='addattr')
 option: Flag('all', autofill=True, cli_name='all', default=False)
@@ -2431,6 +2433,7 @@ option: Str('ipaassignedidview?', autofill=False)
 option: Bool('ipakrbokasdelegate?', autofill=False, cli_name='ok_as_delegate')
 option: Bool('ipakrbrequirespreauth?', autofill=False, cli_name='requires_pre_auth')
 option: Str('ipasshpubkey*', autofill=False, cli_name='sshpubkey')
+option: Str('krbprincipalauthind*', autofill=False, cli_name='auth_ind')
 option: Str('krbprincipalname?', cli_name='principalname')
 option: Str('l?', autofill=False, cli_name='locality')
 option: Str('macaddress*', autofill=False)
diff --git a/VERSION b/VERSION
index d4d7228edb1e29c8655c058e1e4fb727950aeabc..5c

Re: [Freeipa-devel] [PATCH] 0065, 66: webui: authentication indicators on host page

2016-06-29 Thread Petr Vobornik
On 06/28/2016 04:32 PM, Pavel Vomacka wrote:
> Hello,
> 
> please review attached patches. I moved strings used by authentication
> indicators widget to another dict so the second patch changes strings in
> custom_checkbox widget on service page.
> 
> https://fedorahosted.org/freeipa/ticket/5872
> 


ACK

push should wait on server side.

-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH] 0062, 63: webui: Add button for 'server-del' command

2016-06-29 Thread Petr Vobornik
On 06/24/2016 12:40 PM, Pavel Vomacka wrote:
> Hello,
> 
> please review attached patches, they add 'Delete Server' button.
> 

1. there is a whitespace warning while applying patch 63.

2. It breaks expectation of no_init.
Instead of
  var that = IPA.details_facet(spec);
Use
  var that = IPA.details_facet(spec, no_init);

Then rename:
  that.init_facet = function() {
ẗo
  that.init_servers_facet = function() {

Add there at beginning:
  that.init_details_facet


3. IPA.action and IPA.delete_action has the functionality of
server_delete_action build-in, including redirection to server search
page which is missing in this patch.

Updated patch with the issues fixed is attached.

I did not test final version of the patch because my testing env. died.
-- 
Petr Vobornik
From d0ea026cebcda0e301ed75b4dc53c78675c20ef0 Mon Sep 17 00:00:00 2001
From: Pavel Vomacka <pvoma...@redhat.com>
Date: Fri, 24 Jun 2016 12:08:04 +0200
Subject: [PATCH] Add button for server-del command

WebUI counterpart of: https://fedorahosted.org/freeipa/ticket/5588
---
 install/ui/src/freeipa/topology.js | 65 +-
 install/ui/test/data/ipa_init.json |  4 +++
 ipaserver/plugins/internal.py  |  4 +++
 3 files changed, 72 insertions(+), 1 deletion(-)

diff --git a/install/ui/src/freeipa/topology.js b/install/ui/src/freeipa/topology.js
index a9776a5567adf1dcea7a4e62c89f90a573e9ca66..7e501eb3506587ea653497d2806938890066e4f0 100644
--- a/install/ui/src/freeipa/topology.js
+++ b/install/ui/src/freeipa/topology.js
@@ -222,6 +222,7 @@ return {
 },
 {
 $type: 'details',
+$factory: topology.servers_facet,
 disable_facet_tabs: true,
 sections: [
 {
@@ -253,6 +254,30 @@ return {
 }
 ]
 }
+],
+actions: [
+{
+$factory: IPA.delete_action,
+name: 'server_del',
+method: 'del',
+label: '@i18n:objects.servers.remove_server',
+needs_confirm: true,
+confirm_msg: '@i18n:objects.servers.remove_server_msg',
+confirm_dialog: {
+$factory: IPA.confirm_dialog,
+title: '@i18n:objects.servers.remove_server',
+ok_label: '@i18n:buttons.remove',
+ok_button_class: 'btn btn-danger'
+}
+}
+],
+control_buttons_right: [
+{
+name: 'server_del',
+label: '@i18n:objects.servers.remove_server',
+icon: 'fa-exclamation-circle',
+button_class: 'btn btn-danger'
+}
 ]
 }
 ]
@@ -458,6 +483,45 @@ topology.location_adapter = declare([mod_field.Adapter], {
 }
 });
 
+topology.servers_facet = function(spec, no_init) {
+spec = spec || {};
+
+var that = IPA.details_facet(spec, no_init);
+
+/**
+ * Creates buttons on the right side of facet.
+ */
+that.create_controls = function() {
+
+that.create_control_buttons(that.controls_left);
+that.create_action_dropdown(that.controls_left);
+
+that.control_buttons = that.control_buttons_right;
+that.create_control_buttons(that.controls_right);
+};
+
+/**
+ * Inits right facet buttons.
+ */
+that.init_servers_facet = function() {
+
+that.init_details_facet();
+var buttons_spec = {
+$factory: IPA.control_buttons_widget,
+name: 'control-buttons',
+css_class: 'control-buttons',
+buttons: spec.control_buttons_right
+};
+
+that.control_buttons_right = IPA.build(buttons_spec);
+that.control_buttons_right.init(that);
+};
+
+if (!no_init) that.init_servers_facet();
+
+return that;
+};
+
 topology.serverroles_search_facet = function(spec) {
 
 spec = spec || {};
@@ -1404,7 +1468,6 @@ topology.register = function() {
 ctor: topology.TopologyGraphFacet,
 spec: topology.topology_graph_facet_spec
 });
-
 };
 
 phases.on('registration', topology.register);
diff --git a/install/ui/test/data/ipa_init.json b/install/ui/test/data/ipa_init.json
index 49eca4de9d40a20b1aee593e32071c762ff052d1..c18f78cc316a77da46f14fa2b3bbbabe8ee09208 100644
--- a/install/ui/test/data/ipa_init.json
+++ b/install/ui/test/data/ipa_init.json
@@ -539,6 +539,10 @@
 "label": "Server Roles",
 "label_singular": "Server Role",
 },
+"servers": {
+"remove_server": "Delete Server",
+"remove_server_msg": &quo

Re: [Freeipa-devel] [PATCH] 0064: webui: simplify confirmation messages in confirmation dialogs

2016-06-29 Thread Petr Vobornik
On 06/27/2016 05:50 PM, Pavel Vomacka wrote:
> Hello,
> 
> Please review attached patch which simplifies confirmation messages for
> 'remove cert hold' and 'restore cert' actions.
> 

I'd change:
  You can select a reason from the pull-down list.
To:
  Select a reason from the pull-down list.

-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


  1   2   3   4   5   6   7   8   9   10   >