Integration of OIDC authentication #620
Conversation
- Enables creation and management of virtual machines orchestrated through Jenkins - Provides remote access to these machines via Guacamole
- configuration file - authproviders.ini - list in the provider index provided in the configuration file - if connection from a basic rights provider
- configuration file - authproviders.ini - list in the provider index provided in the configuration file - if connection from a basic rights provider (cherry picked from commit 5ce1bd6)
(cherry picked from commit 08c0c67)
(cherry picked from commit 15cf230)
|
🧙 Sourcery has finished reviewing your pull request! Tips
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
We've reviewed this pull request using the Sourcery rules engine. If you would also like our AI-powered code review then let us know.
| headers = {"Content-Type": "application/x-www-form-urlencoded"} | ||
| headers.update(crumb) |
There was a problem hiding this comment.
suggestion (code-quality): Merge dictionary updates via the union operator [×2] (dict-assign-update-to-union)
| headers = {"Content-Type": "application/x-www-form-urlencoded"} | |
| headers.update(crumb) | |
| headers = {"Content-Type": "application/x-www-form-urlencoded"} | crumb |
| info = domain.info() | ||
|
|
||
| root = ET.fromstring(xml) | ||
| domain_info = { |
There was a problem hiding this comment.
issue (code-quality): Inline variable that is immediately returned (inline-immediately-returned-variable)
| source = disk.find("source") | ||
| if source is not None: | ||
| old_path = source.get("file") | ||
| if old_path.startswith( |
There was a problem hiding this comment.
issue (code-quality): Replace call to format with f-string [×5] (use-fstring-for-formatting)
| "The virtual machine '%s' was successfully created !", name | ||
| ) | ||
|
|
||
| dict = getVMInfo(name) |
There was a problem hiding this comment.
issue (code-quality): Don't assign to builtin variable dict (avoid-builtin-shadow)
Explanation
Python has a number ofbuiltin variables: functions and constants thatform a part of the language, such as
list, getattr, and type(See https://docs.python.org/3/library/functions.html).
It is valid, in the language, to re-bind such variables:
list = [1, 2, 3]However, this is considered poor practice.
- It will confuse other developers.
- It will confuse syntax highlighters and linters.
- It means you can no longer use that builtin for its original purpose.
How can you solve this?
Rename the variable something more specific, such as integers.
In a pinch, my_list and similar names are colloquially-recognized
placeholders.
| "/guacamole/", | ||
| ) | ||
| try: | ||
| identifier = guacapi.get_connection_by_name(name)["identifier"] |
There was a problem hiding this comment.
issue (code-quality): We've found these issues:
- Inline variable that is immediately returned (
inline-immediately-returned-variable) - Use f-string instead of string concatenation (
use-fstring-for-concatenation)
| lines = [] | ||
| for row in ret: | ||
| lines.append(row.toDict()) | ||
|
|
||
| return lines |
There was a problem hiding this comment.
suggestion (code-quality): We've found these issues:
- Convert for loop into list comprehension (
list-comprehension) - Inline variable that is immediately returned (
inline-immediately-returned-variable)
| lines = [] | |
| for row in ret: | |
| lines.append(row.toDict()) | |
| return lines | |
| return [row.toDict() for row in ret] |
| @DatabaseHelper._sessionm | ||
| def getVMs(self, session): | ||
| try: | ||
| results = session.query(Machines).all() |
There was a problem hiding this comment.
issue (code-quality): We've found these issues:
- Use named expression to simplify assignment and conditional (
use-named-expression) - Swap if/else branches (
swap-if-else-branches) - Remove unnecessary else after guard condition (
remove-unnecessary-else)
| @DatabaseHelper._sessionm | ||
| def getVMByName(self, session, name): | ||
| try: | ||
| result = session.query(Machines).filter(Machines.nom == name).first() |
There was a problem hiding this comment.
issue (code-quality): We've found these issues:
- Use named expression to simplify assignment and conditional (
use-named-expression) - Inline variable that is immediately returned (
inline-immediately-returned-variable)
| machine = session.query(Machines).filter(Machines.nom == name).first() | ||
| if machine: |
There was a problem hiding this comment.
suggestion (code-quality): Use named expression to simplify assignment and conditional (use-named-expression)
| machine = session.query(Machines).filter(Machines.nom == name).first() | |
| if machine: | |
| if ( | |
| machine := session.query(Machines) | |
| .filter(Machines.nom == name) | |
| .first() | |
| ): |
| result = session.query(Machines).filter(Machines.nom == name).first() | ||
| if result: | ||
| return True | ||
| else: | ||
| return False | ||
|
|
There was a problem hiding this comment.
suggestion (code-quality): We've found these issues:
- Use named expression to simplify assignment and conditional (
use-named-expression) - Simplify boolean if expression (
boolean-if-exp-identity) - Replace if statement with if expression (
assign-if-exp)
| result = session.query(Machines).filter(Machines.nom == name).first() | |
| if result: | |
| return True | |
| else: | |
| return False | |
| return bool( | |
| result := session.query(Machines) | |
| .filter(Machines.nom == name) | |
| .first() | |
| ) |
No description provided.