GMS API integration - #3
Conversation
f3151c5 to
c699e44
Compare
| sources: false | ||
| nodes: false | ||
| spaces: false | ||
| api_system_for_groups: # link the the GMS API |
There was a problem hiding this comment.
# Link 'to' the GMS API
Can you give an example on how the link should be? e.g., https://myapi.com or myapi.com?
| # | ||
| # Points of attention:: Uses class variables (@@bearer_token) for token caching, | ||
| # which is shared across all instances. Consider thread-safety implications | ||
| # in concurrent environments. |
There was a problem hiding this comment.
Let's use Rails.fetch.cache() for caching instead of @@bearer_token class variable – then you can remove the comment in the code
| def valid_token? | ||
| return false unless defined?(@@bearer_token) | ||
| response = HTTParty.get( | ||
| TeSS::Config.feature['api_system_for_groups'] + "/Group/1", |
There was a problem hiding this comment.
LLM found "SSO Token Thread-Safety & API DoS:
@@bearer_token is not thread-safe. valid_token? DDoS-ing your own external API with /Group/1 remains a major issue."
As discussed, if you know the expiration_time/date from the GMS API, then no need to have valid_token? method anymore
|
We must check live:
If tests about this are not done, do it (+ if relevant to upstream TeSS, do a PR) |
| # GET /search_groups | ||
| # | ||
| # Get all the groups of the space. Requires authorization via SpacePolicy#search_groups?. | ||
| # Used only when Group API System is enabled. |
There was a problem hiding this comment.
| # Used only when Group API System is enabled. | |
| # Used only when Group API System is enabled. | |
| # Triggered in the Spaces form when the admin searches for GMS groups |
| # Groups are primarily used to control access to private Space objects: a | ||
| # private space is only accessible to users belonging to one of the space's | ||
| # associated groups (see ApplicationPolicy#shown?). | ||
| # Used only when Group API System is enabled. |
There was a problem hiding this comment.
This is not true (!?)
| # Used only when Group API System is enabled. | |
| # Used only when Group API System is disabled. |
| # of the group (see the +owner+ attribute, managed for example by | ||
| # GroupsController#sync_owners), which grants additional permissions such | ||
| # as editing or destroying the group (see GroupPolicy#owner?). | ||
| # Used only when Group API System is enabled. |
There was a problem hiding this comment.
Again, not true, we use group_membership only when there is the classic Group in TeSS
| # Used only when Group API System is enabled. | |
| # Used only when Group API System is disabled. |
| has_many :administrators, through: :administrator_roles, source: :user, class_name: 'User' | ||
| has_and_belongs_to_many :groups | ||
|
|
||
| has_and_belongs_to_many :groups unless TeSS::Config.feature['api_system_for_groups'] # Used only when Group API System is enabled. |
There was a problem hiding this comment.
| has_and_belongs_to_many :groups unless TeSS::Config.feature['api_system_for_groups'] # Used only when Group API System is enabled. | |
| has_and_belongs_to_many :groups unless TeSS::Config.feature['api_system_for_groups'] # Used only when Group API System is disabled. |
| has_many :users, through: :group_memberships | ||
| # The users belonging to this group, through #group_memberships. | ||
| has_many :users, through: :group_memberships | ||
| end |
There was a problem hiding this comment.
this end closes the unless statement
| # The spaces this group grants access to. | ||
| has_and_belongs_to_many :spaces |
There was a problem hiding this comment.
There is an asymmetry with what's written in space.rb, if the space HABTM relation is disabled, it should be as well here in group
| # The spaces this group grants access to. | |
| has_and_belongs_to_many :spaces | |
| # The spaces this group grants access to. | |
| has_and_belongs_to_many :spaces | |
| end |
There was a problem hiding this comment.
Use Redis.cache here instead of @@redis
| ) | ||
|
|
||
| token = response.parsed_response["access_token"] | ||
| Rails.cache.write("api_service:bearer_token", token, expires_in: (token['expires_in'] - 5).minutes) |
There was a problem hiding this comment.
Cool this is better now!
|
|
||
| <% if is_admin || is_curator || is_owner_in_any_group %> | ||
|
|
||
| <% if !TeSS::Config.feature['api_system_for_groups'] && (is_admin || is_curator || is_owner_in_any_group) %> |
There was a problem hiding this comment.
Note: Even if curator doesn't really care about the group, I understand we leave it like that as it is the case for the other <li> involving is_admin || is_curator
Co-authored-by: Kenneth Rioja <59597207+kennethrioja@users.noreply.github.com>
…ivate-spaces into api-integration
To address in TeSS upstream:
To address in documentations:
|
Co-authored-by: Kenneth Rioja <59597207+kennethrioja@users.noreply.github.com>
…ivate-spaces into api-integration
|
Co-authored-by: Kenneth Rioja <59597207+kennethrioja@users.noreply.github.com>
…ivate-spaces into api-integration
|
Can you fetch hep-training:api-integration-update and cherry-pick the two last commits? (For some reason I couldn't push them to your forked repo branch git remote add hep-training https://github.com/hep-training/TeSS.git
git fetch hep-training
git checkout api-integration
git cherry-pick 872b7d2781043caf5176f0f2a666f7a4b18456b3
git cherry-pick dcbae222827afd8524ea8b6d01d623e5822bf718
git pushThanks! |

Summary of changes
Motivation and context
Using the CERN SSO system for private spaces.
Checklist
[x] I have read and followed the CONTRIBUTING guide.
[x] I confirm that I have the authority necessary to make this contribution on behalf of its copyright owner and agree to license it to the TeSS codebase under the BSD license.