Add SciTokens authorization #75

duncan-brown · 2020-11-12T22:23:23Z

This pull requests adds authorization using SciTokens to the SegDB WSGI server. The SciToken should be passed in the https authorization header as

Authorization: Bearer serialized_token_text

where serialized_token_text is a serialized SciToken generated by a the issuer specified in scitokens_issuer in Constants.py. The SegDB server and the SciToken issuer need to agree on the audience for tokens, which is specified in scitokens_audience in Constants.py.

GET access to obtain segments is given to SegDB if the token scope is read:/DQSegDB and PUT and PATCH access to insert and update segments is given if the token scope is write:/DQSegDB.

If the request does not contain a valid SciToken, or the token cannot be deserialized for the right audience, the server will silently fail over to trying X509 authentication. If a SciToken is provided with the correct audience, the server will fail if the token does not contain the correct scope for the requested action and will not fall through to X509 authentication.

One this patch is merged, the SciTokens library must be installed on the server with yum -y install python2-scitokens. This has been added to the install scripts.

WSGI must to be configured to pass the authorization headers to the python layer with WSGIPassAuthorization On in wsgi.conf. See https://modwsgi.readthedocs.io/en/develop/configuration-directives/WSGIPassAuthorization.html for details. I think I did this correctly in the cit_install_script.sh and install_script.sh files, but in cit_install_script_sl7update.sh the file wsgi.conf seems to be coming from some cached location, so that file may need to be fixed separately.

In addition, the following configuration values need to be set to their production settings before deployment:

scitokens_issuer = 'https://test.cilogon.org'
scitokens_audience = 'segments.ligo.org'

The scitokens_issuer should be the URL of the scitokens issuer against which scitokens should be validated. The scitokens_audience should be the audience that is agreed for the SegDB server(s).

This patch hard codes the scitoken scopes as read:/DQSegSB for GET access to the segment database and write:/DQSegSB for PUT/PATCH access to the segment database. These scopes need to be agreed collaboration-wide. It might also make sense to make them configuration variables as well, rather than being hard coded.

Scitokens caches keys in a cache directory which is set in the configuration file to

scitokens_cache_dir = '/var/cache/httpd'

This directory should exist and have read/write by the DQSegDB server process, or should be changed to another suitable location.

coveralls · 2020-11-12T23:38:58Z

Coverage decreased (-0.4%) to 17.424% when pulling f5ef25a on duncan-brown:scitokens into e829e29 on ligovirgo:master.

duncan-brown · 2020-12-08T16:21:27Z

dqsegdb/urifunctions.py

+    if token_data:
+        token_enforcer = scitokens.Enforcer(tok['iss'], audience=aud)
+        if token_enforcer.test(token, authz, path) is False:
+            raise ValueError('SciToken does not have read:/DQSegDB scope')


Need to generate error message based on passed scope

codecov-commenter · 2021-06-30T14:58:25Z

Codecov Report

Merging #75 (dd29042) into master (738718f) will decrease coverage by 0.28%.
The diff coverage is 4.08%.

@@            Coverage Diff             @@
##           master      #75      +/-   ##
==========================================
- Coverage   11.99%   11.70%   -0.29%     
==========================================
  Files           5        5              
  Lines        1017     1059      +42     
==========================================
+ Hits          122      124       +2     
- Misses        895      935      +40

Flag	Coverage Δ
Linux	`11.70% <4.08%> (-0.29%)`	⬇️
macOS	`11.70% <4.08%> (-0.29%)`	⬇️
python2.7	`11.70% <4.08%> (-0.29%)`	⬇️
python3.5	`?`
python3.6	`?`
python3.7	`?`
python3.8	`?`
python3.9	`?`

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
dqsegdb/urifunctions.py	`13.63% <4.08%> (-3.33%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 738718f...dd29042. Read the comment docs.

duncanmmacleod · 2021-06-30T15:16:19Z

server/src/LDBDWAuth.py

+    def __init__(self):
+        self.admin = Admin.AdminHandle()
+        self.constant = Constants.ConstantsHandle()
+        os.environ['XDG_CACHE_HOME'] = self.constant.scitokens_cache_dir


Why is this needed?

duncanmmacleod · 2021-06-30T15:17:48Z

server/src/LDBDWAuth.py

+            r = [200]
+        # Otherwise, using HTTPS.
+        else:


Can this just return [200] then we can drop the else and deindent the rest of this (very long) function.

Suggested change

r = [200]

# Otherwise, using HTTPS.

else:

return [200]

# Otherwise, using HTTPS.

duncanmmacleod · 2021-06-30T15:19:20Z

server/src/LDBDWAuth.py

+                r = self.admin.log_and_set_http_code(401, c, req_method, "SciToken signature has expired", full_uri)
+                return r


Suggested change

r = self.admin.log_and_set_http_code(401, c, req_method, "SciToken signature has expired", full_uri)

return r

return self.admin.log_and_set_http_code(401, c, req_method, "SciToken signature has expired", full_uri)

duncanmmacleod · 2021-06-30T15:19:33Z

server/src/LDBDWAuth.py

+                r = self.admin.log_and_set_http_code(401, c, req_method, "Invalid audience for SciToken", full_uri)
+                return r


Suggested change

r = self.admin.log_and_set_http_code(401, c, req_method, "Invalid audience for SciToken", full_uri)

return r

return self.admin.log_and_set_http_code(401, c, req_method, "Invalid audience for SciToken", full_uri)

duncanmmacleod · 2021-06-30T15:21:39Z

server/src/application.py

-            res = ldbdwauth.check_authorization_gridmap(environ, environ['REQUEST_METHOD'], environ['REQUEST_URI'], False)
+            try:
+                res = ldbdsauth.check_authorization_scitoken(environ, environ['REQUEST_METHOD'], environ['REQUEST_URI'], False)
+            except:


Please don't use empty except, I presume this should only protect against KeyError which indicates that no token was given

Suggested change

except:

except KeyError:

duncanmmacleod · 2021-06-30T15:22:32Z

server/src/application.py

-            res = ldbdwauth.check_authorization_gridmap(environ, environ['REQUEST_METHOD'], environ['REQUEST_URI'], True)
+            try:
+                res = ldbdsauth.check_authorization_scitoken(environ, environ['REQUEST_METHOD'], environ['REQUEST_URI'], True)
+            except:


as above

Suggested change

except:

except KeyError:

robertbruntz · 2021-07-01T14:08:03Z

@duncan-brown - For anyone who wasn't in this meeting and institutional memory, could you list the elements that need to be modified before we could deploy this?

duncan-brown · 2021-07-01T16:52:43Z

@robertbruntz I updated the main text in the pull request with the items that need to be configured. It might also be a good idea to make the scopes a configurable option in the ini file, rather than hard coded, so I noted that as well.

robertbruntz · 2023-03-20T22:56:31Z

This PR was split into 2 MRs on LIGO GitLab:

"Add SciTokens authorization" - https://git.ligo.org/computing/dqsegdb/client/-/merge_requests/75 - client code, which also contains all of the main text from this PR
"Add Scitokens authorization" - https://git.ligo.org/computing/dqsegdb/server/-/merge_requests/1 - server code, with only brief initial text

All additional work and comments should be in those repos.

duncan-brown force-pushed the scitokens branch from 309c447 to f5ef25a Compare December 4, 2020 22:10

duncan-brown commented Dec 8, 2020

View reviewed changes

duncan-brown added 8 commits June 30, 2021 10:56

add scitokens auth with fallback to x509

d01075a

install scitokens and configure wsgi

93cd7a4

set cache path and use demo server

d813481

clean up error handling

063824f

change issuer to CILogon test server

fb4cccd

log scope if not valid

5aca57a

fix gpl and remove slash from scitoken issuer

eff4585

add scitoken support to client

dd29042

duncan-brown force-pushed the scitokens branch from f5ef25a to dd29042 Compare June 30, 2021 14:57

duncanmmacleod reviewed Jun 30, 2021

View reviewed changes

duncanmmacleod marked this pull request as draft July 1, 2021 14:49

robertbruntz closed this Mar 20, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add SciTokens authorization #75

Add SciTokens authorization #75

duncan-brown commented Nov 12, 2020 •

edited

coveralls commented Nov 12, 2020 •

edited

duncan-brown Dec 8, 2020

codecov-commenter commented Jun 30, 2021 •

edited

duncanmmacleod Jun 30, 2021

duncanmmacleod Jun 30, 2021

duncanmmacleod Jun 30, 2021

duncanmmacleod Jun 30, 2021

duncanmmacleod Jun 30, 2021

duncanmmacleod Jun 30, 2021

robertbruntz commented Jul 1, 2021

duncan-brown commented Jul 1, 2021

robertbruntz commented Mar 20, 2023

		r = self.admin.log_and_set_http_code(401, c, req_method, "SciToken signature has expired", full_uri)
		return r

	r = self.admin.log_and_set_http_code(401, c, req_method, "SciToken signature has expired", full_uri)
	return r
	return self.admin.log_and_set_http_code(401, c, req_method, "SciToken signature has expired", full_uri)

Add SciTokens authorization #75

Add SciTokens authorization #75

Conversation

duncan-brown commented Nov 12, 2020 • edited

coveralls commented Nov 12, 2020 • edited

duncan-brown Dec 8, 2020

Choose a reason for hiding this comment

codecov-commenter commented Jun 30, 2021 • edited

Codecov Report

duncanmmacleod Jun 30, 2021

Choose a reason for hiding this comment

duncanmmacleod Jun 30, 2021

Choose a reason for hiding this comment

duncanmmacleod Jun 30, 2021

Choose a reason for hiding this comment

duncanmmacleod Jun 30, 2021

Choose a reason for hiding this comment

duncanmmacleod Jun 30, 2021

Choose a reason for hiding this comment

duncanmmacleod Jun 30, 2021

Choose a reason for hiding this comment

robertbruntz commented Jul 1, 2021

duncan-brown commented Jul 1, 2021

robertbruntz commented Mar 20, 2023

duncan-brown commented Nov 12, 2020 •

edited

coveralls commented Nov 12, 2020 •

edited

codecov-commenter commented Jun 30, 2021 •

edited