ansible-community / ara

ARA Records Ansible and makes it easier to understand and troubleshoot.
https://ara.recordsansible.org
GNU General Public License v3.0
1.84k stars 170 forks source link

Remote auth not working #123

Closed viktorkrivak closed 4 years ago

viktorkrivak commented 4 years ago

What component is this about ?

ara-server, mainly rest-api

What is your ARA installation like ?

Found in version 1.3.2 (from debian package) but same code is in master

What is happening ?

When EXTERNAL_AUTH is enabled, only admin and healthcheck working properly. /api and index return 401

What should be happening ?

Everything should be working same as it internal auth is used.

Main problem is in server.settings because rest_framework use BasicAuth even if EXTERNAL_AUTH is set. This patch should fix issue

diff --git a/ara/server/settings.py b/ara/server/settings.py
index 252fa98..b4691d1 100644
--- a/ara/server/settings.py
+++ b/ara/server/settings.py
@@ -214,6 +214,12 @@ ROOT_URLCONF = "ara.server.urls"
 APPEND_SLASH = False

 PAGE_SIZE = settings.get("PAGE_SIZE", 100)
+
+if EXTERNAL_AUTH:
+    REST_FRAMEWORK_AUTH = ('rest_framework.authentication.RemoteUserAuthentication',)
+else:
+    REST_FRAMEWORK_AUTH = ('rest_framework.authentication.BasixAuthentication',)
+
 REST_FRAMEWORK = {
     "DEFAULT_PAGINATION_CLASS": "rest_framework.pagination.LimitOffsetPagination",
     "PAGE_SIZE": PAGE_SIZE,
@@ -227,7 +233,7 @@ REST_FRAMEWORK = {
         "rest_framework.parsers.FormParser",
         "rest_framework.parsers.MultiPartParser",
     ),
-    "DEFAULT_AUTHENTICATION_CLASSES": ("rest_framework.authentication.BasicAuthentication",),
+    "DEFAULT_AUTHENTICATION_CLASSES": REST_FRAMEWORK_AUTH ,
     "DEFAULT_PERMISSION_CLASSES": ("ara.api.auth.APIAccessPermission",),
     "TEST_REQUEST_DEFAULT_FORMAT": "json",
     "UNICODE_JSON": False,

NOTE: sorry If this is not a proper way to post patch. There are no pull request so I have no idea where to put this.

dmsimard commented 4 years ago

Hey @viktorkrivak and thanks for the issue. Your assessment of the problem and the patch makes sense at first glance.

If you'd like to submit the patch, there are unfortunately a few hoops to jump through for your first patch (see docs) but your contribution would be appreciated and I would be more than happy to help you get started on Slack or IRC.

Back to the issue, how are you using EXTERNAL_AUTH right now ? I'd like to be able to reproduce the issue and test the patch.

Somewhat related: I've noticed just now that it's missing from the docs and so I created an issue so we don't forget about it: https://github.com/ansible-community/ara/issues/124

viktorkrivak commented 4 years ago

Hi sorry, I didn't notice that part of documentation about gerrit. I'll send patch the correct way. Also I'll write some notes about EXTERNAL_AUTH to #124 . But it'll take some time because I use apache connected to LDAP so it is unnecessary complex setup for documentation, so I need to simplify it.

dmsimard commented 4 years ago

Hi sorry, I didn't notice that part of documentation about gerrit. I'll send patch the correct way.

:tada:

Also I'll write some notes about EXTERNAL_AUTH to #124 . But it'll take some time because I use apache connected to LDAP so it is unnecessary complex setup for documentation, so I need to simplify it.

That's great! Let me know if I can help!

keuko commented 4 years ago

Hi, I'm debian maintainer for ara packages. Would you please let me know when this will be merged ? I would like to upload new version and want to be sure that this will go to upstream.

dmsimard commented 4 years ago

Hey @viktorkrivak, were you still interested in sending the patch ?

Thanks !

keuko commented 4 years ago

Hey @viktorkrivak, were you still interested in sending the patch ?

Thanks !

Well, Viktor is my colleguage :) , I think he already sent a patch above ... But you probably meant Viktor should send pull request , right ?

dmsimard commented 4 years ago

@keuko OH! Yes, it would be great if he could send it through gerrit. We aren't set up for pull requests yet :( I mentioned the docs in a comment above.

keuko commented 4 years ago

@dmsimard , ok I'm going to process it...

dmsimard commented 4 years ago

Much appreciated ! Thank you and let me know if you need a hand.

keuko commented 4 years ago

Much appreciated ! Thank you and let me know if you need a hand.

Here it is :) ! https://review.opendev.org/746140 , let me know if I need to change something.

dmsimard commented 4 years ago

Much appreciated ! Thank you and let me know if you need a hand.

Here it is :) ! https://review.opendev.org/746140 , let me know if I need to change something.

Looks good! I had to rebase your patch for an unrelated failure and left a comment -- should be good to merge afterwards. Thanks.

dmsimard commented 4 years ago

@keuko there you go :tada:

1.4.3 was released earlier this week and the next release is likely going to be 1.5 with the new CLI work so this is where it would land.

From a packaging standpoint, there is a minor change in yaml library (see https://src.fedoraproject.org/rpms/ara/c/2527ca0fbd214dc6269ef8c15e4e97eb20636b8f?branch=master) and the addition of cliff (same framework as openstackclient) for 1.5 which should already be in debian.

I'll close this issue since the change has landed but note that I'd also like to land the documentation from https://github.com/ansible-community/ara/issues/124 for EXTERNAL_AUTH in time for 1.5.

Let me know if you have any questions !

keuko commented 4 years ago

Hi David,

I've already uploaded new debian package do debian/SID ( with patch also ). I can do the work from #124 https://github.com/ansible-community/ara/issues/124 , but I hate docs :D , so please, could you give me a direction how to deal with it ? I mean, what should be included and where ? Do I need also write playbook for it ?

Thank you for your quick responses :)

Cheers, Michal Arbet ( kevko )

št 13. 8. 2020 o 21:21 David Moreau Simard notifications@github.com napísal(a):

@keuko https://github.com/keuko there you go 🎉

1.4.3 was released earlier this week and the next release is likely going to be 1.5 with the new CLI https://ara.readthedocs.io/en/latest/cli.html work so this is where it would land.

From a packaging standpoint, there is a minor change in yaml library (see https://src.fedoraproject.org/rpms/ara/c/2527ca0fbd214dc6269ef8c15e4e97eb20636b8f?branch=master) and the addition of cliff (same framework as openstackclient) for 1.5 which should already be in debian.

I'll close this issue since the change has landed but note that I'd also like to land the documentation from #124 https://github.com/ansible-community/ara/issues/124 for EXTERNAL_AUTH in time for 1.5.

Let me know if you have any questions !

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ansible-community/ara/issues/123#issuecomment-673664641, or unsubscribe https://github.com/notifications/unsubscribe-auth/AF4UEEIHDAXO5V3NUJP3D63SAQ4NLANCNFSM4MNN47UQ .

dmsimard commented 4 years ago

Hey @keuko,

Do you have a link for the debian packaging files ? It would be nice to write it down somewhere along with the packaging files for Fedora and SUSE.

I empathize with your feelings for docs, I think we need to do a good docs refresh in general but I won't bother you with that :)

For now if you'd like to add the external auth docs, the best fit would be in the "ARA API Server authentication and security" section (or here in the git repo). Don't be scared to take a first stab at it and we can iterate together during code review if need be.

The support for it in the Ansible roles aren't required. They're nice to have, though, because we use the roles during integration tests so we not only test the roles themselves but also the different ways to deploy and configure ara. I can take care of this eventually.

Note that the roles haven't been removed from this repository just yet but we're in the process of moving those to a collection: https://github.com/ansible-community/ara-collection

dmsimard commented 4 years ago

Ah, I found the debian packaging work through https://tracker.debian.org/pkg/python-ara. Thank you very much for your work on that !

keuko commented 4 years ago

Hi david,

I've added a review for docs ( usage of external_auth ) here -> https://review.opendev.org/746516 . Could you check and leave some comments ?

Thank you !

Cheers, Michal Arbet ( kevko )

pi 14. 8. 2020 o 16:26 David Moreau Simard notifications@github.com napísal(a):

Ah, I found the debian packaging work through https://tracker.debian.org/pkg/python-ara. Thank you very much for your work on that !

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ansible-community/ara/issues/123#issuecomment-674100713, or unsubscribe https://github.com/notifications/unsubscribe-auth/AF4UEEK2RYN2D5H72XB4Y3LSAVCP7ANCNFSM4MNN47UQ .

dmsimard commented 3 years ago

Hey @keuko and @viktorkrivak, I just wanted to let you know that external auth was released in 1.5 today. You can see the full release notes here and the highlights on the blog: https://ara.recordsansible.org/blog/2020/09/23/announcing-the-release-of-ara-1.5.0/

Thanks for your patience and your help !