Skip to content

Commit

Permalink
Merge pull request #7841 from ckan/salsadigitalauorg-7408-Authenticat…
Browse files Browse the repository at this point in the history
…ion-header-name

[#7408] Authentication header name follow-up
  • Loading branch information
pdelboca committed Nov 1, 2023
2 parents 2c8a888 + 48c19d9 commit 0996b54
Show file tree
Hide file tree
Showing 11 changed files with 69 additions and 41 deletions.
21 changes: 21 additions & 0 deletions changes/7841.migration
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
The configuration option to customize the authorization header name has been renamed to :ref:`apitoken_header_name` from ``apikey_header_name``.

Tests performing requests using the test client should authenticate users sending the default ``Authorization`` header with a valid token, as opposed to sending the user name in ``environ_overrides`` (or the older ``extra_environ``).

Before::

def test_dataset_new(app):

user = factories.User()

app.get(url_for("dataset.new"), environ_overrides={"REMOTE_USER": user["name"]})


After::

def test_dataset_new(app):

user = factories.UserWithToken()

app.get(url_for("dataset.new"), headers={"Authorization": user["token"]})

15 changes: 8 additions & 7 deletions ckan/config/config_declaration.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -409,13 +409,14 @@ groups:
.. warning:: This setting should not have a trailing / on the end.
- key: apikey_header_name
default: X-CKAN-API-Key
example: API-KEY
description: |
This allows another http header to be used to provide the CKAN API
key. This is useful if network infrastructure blocks the
Authorization header and ``X-CKAN-API-Key`` is not suitable.
- key: apitoken_header_name
default: Authorization
legacy_key: apikey_header_name # since v2.10.0
example: X-CKAN-API-TOKEN
description: |
This allows to customize the name of the HTTP header used to provide the CKAN API
token. This is useful in some scenarios where using the default ``Authorization`` one
causes problems.
- key: ckan.cache_expires
default: 0
Expand Down
3 changes: 0 additions & 3 deletions ckan/lib/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,6 @@

log = logging.getLogger(__name__)

APIKEY_HEADER_NAME_KEY = 'apikey_header_name'
APIKEY_HEADER_NAME_DEFAULT = 'X-CKAN-API-Key'


def abort(status_code: int,
detail: str = '',
Expand Down
6 changes: 3 additions & 3 deletions ckan/tests/controllers/test_package.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,11 +88,11 @@ def test_new_indexerror(self, app, user):

def test_change_locale(self, app, user):
url = url_for("dataset.new")
env = {"Authorization": user["token"]}
res = app.get(url, extra_environ=env)
headers = {"Authorization": user["token"]}
res = app.get(url, headers=headers)
# See https://github.com/python-babel/flask-babel/issues/214
refresh_babel()
res = app.get("/de/dataset/new", extra_environ=env)
res = app.get("/de/dataset/new", headers=headers)
assert helpers.body_contains(res, "Datensatz")

@pytest.mark.ckan_config("ckan.auth.create_unowned_dataset", "false")
Expand Down
32 changes: 26 additions & 6 deletions ckan/tests/lib/test_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ def test_cors_config_origin_allow_all_true_with_origin(app):
)
assert (
response_headers["Access-Control-Allow-Headers"]
== "X-CKAN-API-KEY, Authorization, Content-Type"
== "Authorization, Content-Type"
)


Expand Down Expand Up @@ -176,7 +176,7 @@ def test_cors_config_origin_allow_all_false_with_whitelisted_origin(app):
)
assert (
response_headers["Access-Control-Allow-Headers"]
== "X-CKAN-API-KEY, Authorization, Content-Type"
== "Authorization, Content-Type"
)


Expand Down Expand Up @@ -210,7 +210,7 @@ def test_cors_config_origin_allow_all_false_with_multiple_whitelisted_origins(
)
assert (
response_headers["Access-Control-Allow-Headers"]
== "X-CKAN-API-KEY, Authorization, Content-Type"
== "Authorization, Content-Type"
)


Expand Down Expand Up @@ -314,7 +314,7 @@ def test_cors_config_origin_allow_all_true_with_origin_2(app):
)
assert (
response_headers["Access-Control-Allow-Headers"]
== "X-CKAN-API-KEY, Authorization, Content-Type"
== "Authorization, Content-Type"
)


Expand Down Expand Up @@ -367,7 +367,7 @@ def test_cors_config_origin_allow_all_false_with_whitelisted_origin_2(app):
)
assert (
response_headers["Access-Control-Allow-Headers"]
== "X-CKAN-API-KEY, Authorization, Content-Type"
== "Authorization, Content-Type"
)


Expand Down Expand Up @@ -403,7 +403,27 @@ def test_cors_config_origin_allow_all_false_with_multiple_whitelisted_origins_2(
)
assert (
response_headers["Access-Control-Allow-Headers"]
== "X-CKAN-API-KEY, Authorization, Content-Type"
== "Authorization, Content-Type"
)


@pytest.mark.ckan_config("ckan.cors.origin_allow_all", "true")
@pytest.mark.ckan_config("ckan.site_url", "http://test.ckan.org")
@pytest.mark.ckan_config("apitoken_header_name", "X-CKAN-API-TOKEN")
@pytest.mark.ckan_config("ckan.plugins", "test_blueprint_plugin")
@pytest.mark.usefixtures("with_plugins")
def test_cors_config_custom_auth_header(app):
"""
When using a custom value for the auth header, this should be returned
in the Access-Control-Allow-Headers header in the response.
"""
request_headers = {"Origin": "http://thirdpartyrequests.org"}
response = app.get("/simple_url", headers=request_headers)
response_headers = dict(response.headers)

assert (
response_headers["Access-Control-Allow-Headers"]
== "X-CKAN-API-TOKEN, Content-Type"
)


Expand Down
18 changes: 4 additions & 14 deletions ckan/views/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ def set_cors_headers_for_response(response: Response) -> Response:
response.headers['Access-Control-Allow-Methods'] = \
'POST, PUT, GET, DELETE, OPTIONS'
response.headers['Access-Control-Allow-Headers'] = \
'X-CKAN-API-KEY, Authorization, Content-Type'
f'{config.get("apitoken_header_name")}, Content-Type'

return response

Expand Down Expand Up @@ -118,23 +118,13 @@ def identify_user() -> Optional[Response]:


def _get_user_for_apitoken() -> Optional[model.User]: # type: ignore
apitoken_header_name = config.get("apikey_header_name")

apitoken_header_name = config.get("apitoken_header_name")
apitoken: str = request.headers.get(apitoken_header_name, u'')
if not apitoken:
apitoken = request.environ.get(apitoken_header_name, u'')
if not apitoken:
# For misunderstanding old documentation (now fixed).
apitoken = request.environ.get(u'HTTP_AUTHORIZATION', u'')
if not apitoken:
apitoken = request.environ.get(u'Authorization', u'')
# Forget HTTP Auth credentials (they have spaces).
if u' ' in apitoken:
apitoken = u''

if not apitoken:
return None
apitoken = str(apitoken)
log.debug(u'Received API Token: %s' % apitoken)
log.debug(f'Received API Token: {apitoken[:10]}[...]')

user = api_token.get_user_from_token(apitoken)

Expand Down
2 changes: 1 addition & 1 deletion ckanext/reclineview/assets/vendor/ckan.js/ckan.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ if (isNodeModule) {
my.Client.prototype._ajax = function(options, cb) {
options.headers = options.headers || {};
if (this.apiKey) {
options.headers['X-CKAN-API-KEY'] = this.apiKey;
options.headers['Authorization'] = this.apiKey;
}

var csrf_field = $('meta[name=csrf_field_name]').attr('content');
Expand Down
6 changes: 3 additions & 3 deletions doc/api/index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -271,9 +271,9 @@ but its use is discouraged as they are not as secure as tokens and are limited t
Support for legacy API keys will be removed in future CKAN versions.


To provide your API token in an HTTP request, include it in either an
``Authorization`` or ``X-CKAN-API-Key`` header. (The name of the HTTP header
can be configured with the ``apikey_header_name`` option in your CKAN
To provide your API token in an HTTP request, include it in an
``Authorization`` header. (The name of the HTTP header
can be configured with the :ref:``apitoken_header_name`` option in your CKAN
configuration file.)

For example, to ask whether or not you're currently following the user
Expand Down
2 changes: 1 addition & 1 deletion doc/extensions/tutorial.rst
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ receive an ``Authorization Error`` from CKAN::

$ http 127.0.0.1:5000/api/3/action/group_create Authorization:*** name=my_group
HTTP/1.0 403 Forbidden
Access-Control-Allow-Headers: X-CKAN-API-KEY, Authorization, Content-Type
Access-Control-Allow-Headers: Authorization, Content-Type
Access-Control-Allow-Methods: POST, PUT, GET, DELETE, OPTIONS
Access-Control-Allow-Origin: *
Cache-Control: no-cache
Expand Down
2 changes: 1 addition & 1 deletion doc/maintaining/filestore.rst
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ To create a new resource and upload a file to it using the Python library
import requests
requests.post('http://0.0.0.0:5000/api/action/resource_create',
data={"package_id":"my_dataset"},
headers={"X-CKAN-API-Key": "21a47217-6d7b-49c5-88f9-72ebd5a4d4bb"},
headers={"Authorization": "21a47217-6d7b-49c5-88f9-72ebd5a4d4bb"},
files=[('upload', open('/path/to/file/to/upload.csv', 'rb'))])
(Requests automatically sends a ``multipart-form-data`` heading when you use the
Expand Down
3 changes: 1 addition & 2 deletions test-core.ini
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,7 @@ package_new_return_url = http://localhost/dataset/<NAME>?test=new
package_edit_return_url = http://localhost/dataset/<NAME>?test=edit
ckan.extra_resource_fields = alt_url

# Change API key HTTP header to something non-standard.
apikey_header_name = X-Non-Standard-CKAN-API-Key
apitoken_header_name = Authorization

ckan.plugins =
# ckan.views.default_views =
Expand Down

0 comments on commit 0996b54

Please sign in to comment.