Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(docs): refresh agama lab quick start guide with respect to the latest Agama Lab version #10426

Open
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

ossdhaval
Copy link
Contributor

Prepare


Description

Target issue

closes #issue-number-here

Implementation Details


Test and Document the changes

  • Static code analysis has been run locally and issues have been fixed
  • Relevant unit and integration tests have been added/updated
  • Relevant documentation has been updated if any (i.e. user guides, installation and configuration guides, technical design docs etc)

Please check the below before submitting your PR. The PR will not be merged if there are no commits that start with docs: to indicate documentation changes or if the below checklist is not selected.

  • I confirm that there is no impact on the docs due to the code changes in this PR.

@ossdhaval ossdhaval self-assigned this Dec 16, 2024
@mo-auto mo-auto added area-documentation Documentation needs to change as part of issue or PR comp-agama Touching folder /agama comp-docs Touching folder /docs kind-bug Issue or PR is a bug in existing functionality labels Dec 16, 2024
Copy link
Contributor

@SafinWasi SafinWasi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs changes, lots of links point to private assets.

docs/assets/agamalab-flow-passwd-edit-assignment-uid.png Outdated Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

creds.username and creds.password should be separate arguments, not one argument separated by spaces. This can be done by typing creds.username, hitting Enter, then typing creds.password.
image

to test this project.

- [Install Jans Tarp browser plug-in](https://github.com/JanssenProject/jans/tree/main/demos/jans-tarp#installation-in-browser)
- Follow [these instructions]() to test using the Janssen Server
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blank link

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still blank link

![image](../../../assets/agama-login.png)
- Upon entering a valid user-name and password, the tarp should present a page
with access token.
![image](../../../assets/successful-tarp-auth-screen.png)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs a newline before image, otherwise it breaks the formatting.
image

Signed-off-by: ossdhaval <[email protected]>
Copy link

dryrunsecurity bot commented Dec 23, 2024

DryRun Security Summary

There are no code changes or files modified in the provided text, so no summary or review can be generated.

Expand for full summary

Summary:

There are no code changes provided in the input, so I don't have any specific code changes to review or summarize. As an application security engineer, I would typically review any code changes in a pull request to ensure they do not introduce any security vulnerabilities or unintended consequences. Without any code changes to analyze, I don't have any specific security-related observations to provide. However, I'm ready to review any future code changes that may be submitted.

Files Changed:

There are no files changed in the provided input.

Code Analysis

We ran 9 analyzers against 30 files and 0 analyzers had findings. 9 analyzers had no findings.

View PR in the DryRun Dashboard.

Copy link
Contributor

@SafinWasi SafinWasi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some errors are still not fixed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The arguments here are still wrong. #10426 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"String type" should be unchecked

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is correct. The option selected should be "Flow finished with an error" with a description. Like so:
image

"Flow finished with a variable" is used when you want to return a predefined variable. "false" is not a variable. Technically Finish false is correct according to Agama syntax, but it is better to use descriptive option

// Create new CdiUtil object
cdiUtil = Call io.jans.service.cdi.uti.CdUtil#bean authService
// Empty result object
authResult = "{}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is still incorrect

// Validate username and password
authResult.success = Call cdiUtil#authenticate creds.username creds.password
// Keep the username if the user needs to retry authentication
authResult.uid = "creds.username"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
authResult.uid = "creds.username"
authResult.uid = creds.username

authResult.success = Call cdiUtil#authenticate creds.username creds.password
// Keep the username if the user needs to retry authentication
authResult.uid = "creds.username"
When authResult.uid is true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
When authResult.uid is true
When authResult.success is true

to test this project.

- [Install Jans Tarp browser plug-in](https://github.com/JanssenProject/jans/tree/main/demos/jans-tarp#installation-in-browser)
- Follow [these instructions]() to test using the Janssen Server
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still blank link


To use the flow for authentication in the Janssen Server, the flow needs to be released. Agama Lab
releases the flow and the related artifacts (like template) in the form of a `.gama` file in the GitHub repository.
At this point we have all the components of an Agama project ready, and they are
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As the screenshot shows, all Agama stuff should be on the main branch to create a release. So there should be instructions added to make a PR from agama-lab-policy-designer branch to default branch.

Signed-off-by: ossdhaval <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-documentation Documentation needs to change as part of issue or PR comp-agama Touching folder /agama comp-docs Touching folder /docs kind-bug Issue or PR is a bug in existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants