waiverdb

Clone

#328 Use flask-cors library to support CORS headers
Merged 4 years ago by gnaponie. Opened 4 years ago by lholecek.
lholecek/waiverdb cors  into  master

file modified
+1 
@@ -5,6 +5,7 @@ 
 

  Flask
 

  Flask-RESTful!=0.3.6
 

  Flask-SQLAlchemy
 

+ flask-cors
 

  SQLAlchemy
 

  gssapi
 

  flask-oidc

file modified
-5 
@@ -79,11 +79,6 @@ 
 

  

  

  @pytest.fixture()
 

- def enable_cors(app, monkeypatch):
 

-     monkeypatch.setitem(app.config, 'CORS_URL', 'https://bodhi.fedoraproject.org')
 

- 

- 

- @pytest.fixture()
 

  def enable_permission_mapping(app, monkeypatch):
 

      monkeypatch.setitem(app.config, 'PERMISSION_MAPPING',
 

                          {

file modified
+26 -56 
@@ -638,68 +638,38 @@ 
 

      assert output['auth_method'] == client.application.config['AUTH_METHOD']
 

  

  

- @pytest.mark.usefixtures('enable_cors')
 

- def test_cors_about(client, session):
 

-     r = client.get('/api/v1.0/about')
 

- 

-     assert 'Access-Control-Allow-Origin' in list(r.headers.keys())
 

-     assert 'Access-Control-Allow-Headers' in list(r.headers.keys())
 

-     assert 'Access-Control-Allow-Method' in list(r.headers.keys())
 

-     assert r.headers['Access-Control-Allow-Origin'] == 'https://bodhi.fedoraproject.org'
 

-     assert r.headers['Access-Control-Allow-Headers'] == 'Content-Type'
 

-     assert r.headers['Access-Control-Allow-Method'] == 'POST, OPTIONS'
 

- 

-     output = json.loads(r.get_data(as_text=True))
 

-     assert r.status_code == 200
 

-     assert output['version'] == __version__
 

- 

- 

- def test_no_cors_about(client, session):
 

-     r = client.get('/api/v1.0/about')
 

- 

-     assert 'Access-Control-Allow-Origin' not in list(r.headers.keys())
 

-     assert 'Access-Control-Allow-Headers' not in list(r.headers.keys())
 

-     assert 'Access-Control-Allow-Method' not in list(r.headers.keys())
 

- 

-     output = json.loads(r.get_data(as_text=True))
 

-     assert r.status_code == 200
 

-     assert output['version'] == __version__
 

- 

- 

- @pytest.mark.usefixtures('enable_cors')
 

- def test_cors_waivers(client, session):
 

-     for i in range(0, 3):
 

-         create_waiver(session, subject_type='koji_build', subject_identifier="%d" % i,
 

-                       testcase="case %d" % i, username='foo %d' % i,
 

-                       product_version='foo-%d' % i, comment='bla bla bla')
 

-     r = client.get('/api/v1.0/waivers/')
 

- 

-     assert 'Access-Control-Allow-Origin' in list(r.headers.keys())
 

-     assert 'Access-Control-Allow-Headers' in list(r.headers.keys())
 

-     assert 'Access-Control-Allow-Method' in list(r.headers.keys())
 

-     assert r.headers['Access-Control-Allow-Origin'] == 'https://bodhi.fedoraproject.org'
 

-     assert r.headers['Access-Control-Allow-Headers'] == 'Content-Type'
 

-     assert r.headers['Access-Control-Allow-Method'] == 'POST, OPTIONS'
 

+ def test_cors_good(client, session):
 

+     headers = {
 

+         'Access-Control-Request-Method': 'POST',
 

+         'Access-Control-Request-Headers': 'Content-Type',
 

+         'Origin': 'https://bodhi.fedoraproject.org',
 

+     }
 

+     r = client.options(
 

+         '/api/v1.0/waivers/',
 

+         content_type='Content-Type',
 

+         headers=headers
 

+     )
 

  

-     res_data = json.loads(r.get_data(as_text=True))
 

      assert r.status_code == 200
 

-     assert len(res_data['data']) == 3
 

+     assert r.headers.get('Access-Control-Allow-Origin') == 'https://bodhi.fedoraproject.org'
 

+     assert 'POST' in r.headers.get('Access-Control-Allow-Methods', '').split(', ')
 

  

  

- def test_no_cors_waivers(client, session):
 

-     for i in range(0, 3):
 

-         create_waiver(session, subject_type='koji_build', subject_identifier="%d" % i,
 

-                       testcase="case %d" % i, username='foo %d' % i,
 

-                       product_version='foo-%d' % i, comment='bla bla bla')
 

-     r = client.get('/api/v1.0/waivers/')
 

- 

-     assert 'Access-Control-Allow-Origin' not in list(r.headers.keys())
 

-     assert 'Access-Control-Allow-Headers' not in list(r.headers.keys())
 

-     assert 'Access-Control-Allow-Method' not in list(r.headers.keys())
 

+ def test_cors_bad(client, session):
 

+     headers = {
 

+         'Access-Control-Request-Method': 'POST',
 

+         'Access-Control-Request-Headers': 'Content-Type',
 

+         'Origin': 'localhost',
 

+     }
 

+     r = client.options(
 

+         '/api/v1.0/waivers/',
 

+         content_type='Content-Type',
 

+         headers=headers
 

+     )
 

  

-     res_data = json.loads(r.get_data(as_text=True))
 

      assert r.status_code == 200
 

-     assert len(res_data['data']) == 3
 

+     assert 'Access-Control-Allow-Origin' not in r.headers
 

+     assert 'Access-Control-Allow-Methods' not in r.headers
 

  

  

  @patch('waiverdb.auth.get_user', return_value=('foo', {}))

file modified
+2 
@@ -31,6 +31,7 @@ 
 

  BuildRequires:  python3-sphinxcontrib-issuetracker
 

  BuildRequires:  python3-flask
 

  BuildRequires:  python3-sqlalchemy
 

+ BuildRequires:  python3-flask-cors
 

  BuildRequires:  python3-flask-restful
 

  BuildRequires:  python3-flask-sqlalchemy
 

  BuildRequires:  python3-psycopg2
 
@@ -47,6 +48,7 @@ 
 

  BuildRequires:  python3-six
 

  Requires:       python3-flask
 

  Requires:       python3-sqlalchemy
 

+ Requires:       python3-flask-cors
 

  Requires:       python3-flask-restful
 

  Requires:       python3-flask-sqlalchemy
 

  Requires:       python3-psycopg2

file modified
+15 -14 
@@ -8,6 +8,7 @@ 
 

      from urlparse import urlparse, urlunsplit
 

  

  from flask import Flask, current_app
 

+ from flask_cors import CORS
 

  from flask_migrate import Migrate
 

  from sqlalchemy import event
 

  from sqlalchemy.exc import ProgrammingError
 
@@ -23,6 +24,18 @@ 
 

  from waiverdb.monitor import db_hook_event_listeners
 

  

  

+ def enable_cors(app):
 

+     """
 

+     Enables CORS headers.
 

+     """
 

+     # backward compatibility with old CORS_URL option
 

+     cors_url = app.config.get('CORS_URL')
 

+     if cors_url:
 

+         app.config['CORS_ORIGINS'] = cors_url
 

+ 

+     CORS(app)
 

+ 

+ 

  def load_config(app):
 

      # Load default config, then override that with a config file
 

      if os.getenv('DEV') == 'true':
 
@@ -63,18 +76,6 @@ 
 

      app.config['SQLALCHEMY_DATABASE_URI'] = dburi
 

  

  

- def insert_headers(response):
 

-     """ Insert the CORS headers for the give response if there are any
 

-     configured for the application.
 

-     """
 

-     cors_url = current_app.config.get('CORS_URL')
 

-     if cors_url:
 

-         response.headers['Access-Control-Allow-Origin'] = cors_url
 

-         response.headers['Access-Control-Allow-Headers'] = 'Content-Type'
 

-         response.headers['Access-Control-Allow-Method'] = 'POST, OPTIONS'
 

-     return response
 

- 

- 

  # applicaiton factory http://flask.pocoo.org/docs/0.12/patterns/appfactories/
 

  def create_app(config_obj=None):
 

      app = Flask(__name__)
 
@@ -107,11 +108,11 @@ 
 

      app.add_url_rule('/healthcheck', view_func=healthcheck)
 

      register_event_handlers(app)
 

  

-     app.after_request(insert_headers)
 

- 

      # initialize DB event listeners from the monitor module
 

      app.before_first_request(db_hook_event_listeners)
 

  

+     enable_cors(app)
 

+ 

      return app

file modified
+2 
@@ -62,3 +62,5 @@ 
 

      OIDC_REQUIRED_SCOPE = 'waiverdb_scope'
 

      OIDC_RESOURCE_SERVER_ONLY = True
 

      SUPERUSERS = ['bodhi']
 

+ 

+     CORS_ORIGINS = 'https://bodhi.fedoraproject.org'

Moves the CORS header support to the library.

JIRA: FACTORY-4516
JIRA: FACTORY-4524
Signed-off-by: Lukas Holecek hluk@email.cz

Build d7cff59f400c69cf17b47aace50291d93bb4cea7 FAILED!
Rebase or make new commits to rebuild.

rebased onto 5bd39e560aea206a7b853f7de90faf01acac4046
4 years ago

14:03:35 error: Failed build dependencies:
14:03:35 python3-flask-cors is needed by waiverdb-1.0.1-0.git.18.d7cff59.fc29.noarch

Failed because Jenkins slave didn't have the right dependencies installed. I've updated Jenkins slave with following command and repushed changes.

oc start-build --from-repo=. --commit=cors waiverdb-premerge-jenkins-slave

Does CORS_SUPPORTS_CREDENTIALS need to be set as well to support the authenticated POST request?

Does CORS_SUPPORTS_CREDENTIALS need to be set as well to support the authenticated POST request?

Not sure, works locally with mocked authentication using Firefox. I think this option is only required if cookies are involved.

:thumbsup:

looks good

rebased onto 9946466
4 years ago

Had to remove the config CORS_METHODS = ['POST', 'OPTIONS']. Default is all HTTP methods. Otherwise GET about endpoint with authentication doesn't work.

Had to remove the config CORS_METHODS = ['POST', 'OPTIONS']. Default is all HTTP methods. Otherwise GET about endpoint with authentication doesn't work.

Perhaps you need to add Authorization to the Access-Control-Request-Headers header returned.

Does CORS_SUPPORTS_CREDENTIALS need to be set as well to support the authenticated POST request?

Not sure, works locally with mocked authentication using Firefox. I think this option is only required if cookies are involved.

The documentation says "Credentials are cookies, authorization headers or TLS client certificates" [1]. So I think it maybe necessary.

1 - https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Allow-Credentials

Does CORS_SUPPORTS_CREDENTIALS need to be set as well to support the authenticated POST request?
Not sure, works locally with mocked authentication using Firefox. I think this option is only required if cookies are involved.

The documentation says "Credentials are cookies, authorization headers or TLS client certificates" [1]. So I think it maybe necessary.
1 - https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Allow-Credentials

From library docs:

supports_credentials (bool) –
Allows users to make authenticated requests. If true, injects the Access-Control-Allow-Credentials header in responses. This allows cookies and credentials to be submitted across domains.

I don't think we want to send, for example, Bohdi credentials over to WaiverDB. Am I missing something?

Does CORS_SUPPORTS_CREDENTIALS need to be set as well to support the authenticated POST request?
Not sure, works locally with mocked authentication using Firefox. I think this option is only required if cookies are involved.
The documentation says "Credentials are cookies, authorization headers or TLS client certificates" [1]. So I think it maybe necessary.
1 - https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Allow-Credentials

From library docs:
supports_credentials (bool) –
Allows users to make authenticated requests. If true, injects the Access-Control-Allow-Credentials header in responses. This allows cookies and credentials to be submitted across domains.
I don't think we want to send, for example, Bohdi credentials over to WaiverDB. Am I missing something?

You're right. I misunderstood the documentation

Had to remove the config CORS_METHODS = ['POST', 'OPTIONS']. Default is all HTTP methods. Otherwise GET about endpoint with authentication doesn't work.

Perhaps you need to add Authorization to the Access-Control-Request-Headers header returned.

Access-Control-Request-Headers header is set to * by flask-cors.

Adding 'GET' to CORS_METHODS works. GET requests in waiverdb doesn't require authentication but it's probably better not to block them when they contain Authorization header.

Every comment seems to be addressed. Merging the PR.

Commit 093d946 fixes this pull-request

Pull-Request has been merged by gnaponie
4 years ago

Pull-Request has been merged by gnaponie
4 years ago
Metadata
None
No Tags
Flags
c3i-jenkins
success (100%)
Build passed.
4 years ago
Changes Summary 6
+1 -0
file changed
requirements.txt
+0 -5
file changed
tests/conftest.py
+26 -56
file changed
tests/test_api_v10.py
+2 -0
file changed
waiverdb.spec
+15 -14
file changed
waiverdb/app.py
+2 -0
file changed
waiverdb/config.py
Powered by Pagure 5.13.3
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.