DDFBRA-134 - Setup simple_oauth module #1737
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…t service. In our decorator, we added functionality that checks wether the request path starts with '/graphql' or not. The reason for this is, that right now the isOauth2Request function only checks if the request contains a bearer token. This is also the case when the requests come from the opening hours REST API endpoints, as it submits a library token. By checking for the /graphql in the path, we make sure to only apply the oauth2 validation on the graphql requests.
Dresse
changed the title
…r. This" This reverts commit aa1b405.
spaceo
reviewed
Nov 14, 2024
…skernesdigitalebibliotek/dpl-cms into DDFBRA-138-setup-simple-oauth-module
…in the simple_oauth module. The update hook is also called from the install hook as we want to generate the keys on both new and old sites. Updated the simple_oauth config to look for the keys in the correct directory.
spaceo
reviewed
Nov 20, 2024
Comment on lines
36
to
37
| $path = '../web/sites/default/files/simple_oauth_keys'; | ||
| $file_system->prepareDirectory($path, FileSystemInterface::CREATE_DIRECTORY); |
There was a problem hiding this comment.
The files need to be written to a private directory so the are not accessible publicly.
The best think would be that we could write in a mount point that is not accesible at all by the web.
We could look into that later.
But for now we need to make sure that the directory that we create in sites/default/files is protected.
You need to investigate if the path to the private dir is set in settings, otherwise you shoul be able to set it via:
$settings['file_private_path'] = SOME_ABSOLUTE_PATH_TO_THE_DIRECTORY
When you need the path to the dir you should be able to do so by calling:
$private_files_path = \Drupal::service('file_system')->realpath('private://');
There was a problem hiding this comment.
I have now added the $settings['file_private_path'] in our settings.php.
This points to sites/default/files/private.
When generating the keys they are now being placed within that folder.
Is it enough to do this for now?
spaceo
requested changes
Nov 20, 2024
The private folder is currently placed within /sites/default/files folder. This is not idea, as the private files folder should be placed outside of the web root folder. We will look into changing this in the future. Changed the install hook to generate the keys in the private directory. Also changed the simple_oauth configuration to look in the new directory for the private and public keys.
Dresse
force-pushed
the
DDFBRA-138-setup-simple-oauth-module
branch
from
2c9757d to
15ade81
Compare
Dresse
force-pushed
the
DDFBRA-138-setup-simple-oauth-module
branch
from
15ade81 to
de7b67f
Compare
Dresse
force-pushed
the
DDFBRA-138-setup-simple-oauth-module
branch
from
c65ded9 to
2ef6894
Compare
Dresse
force-pushed
the
DDFBRA-138-setup-simple-oauth-module
branch
from
2ef6894 to
57154ac
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Link to issue
https://reload.atlassian.net/browse/DDFBRA-134
Description
This PR adds the simple_oauth module, which we want to use for authenticating when requesting the GraphQL endpoints.
This PR also fixes 2 issue we found during testing of the module:
appliesfunction was using a function (isOauth2Request) for checking wether Requests was OAuth2 requests. The problem however, was that it was just checking to see if a Bearer token was present. This causes problems with the current REST API for fetching opening hours, as those requests would sometime contain the Library token as a Bearer token, and thus the simple_oauth module would try to authenticate the request.To solve this issue, this PR extends the simple_oauth modules functionality by decorating the service that provides the logic for determining if a request is a Oauth2 request. The added logic simple check to see if the request path stats with
/graphql. If this is not the case, we return FALSE.The PR also updates the openapi.json file with the new specification.
Lastly this PR makes sure to generate the public and private keys that the
simple_oauthmodules needs to use when creating tokens. This is done in an update hook, but we also make sure to call the same update hook in the install hook function, as we want to make sure that the keys are generated on both old and new sites. I am unsure if this is the best approach, but we do something similar indpl_updatemodule, and I did not think it was worth using more time on figuring a better way of doing this. We also update the configuration for where the module should look for the generated keys.Other comments
We had a talk about where the public and private keys should be generated. We ended up deciding on placing them in folder called
simple_oauth_keysplaced in the/web/sites/default/filesdirectory.