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

Validates client config #1083

Conversation

atomicgamedeveloper
Copy link
Contributor

@atomicgamedeveloper atomicgamedeveloper commented Nov 29, 2024

Title

Validates client config.

Fixes #658

Type of Change

  • New feature
  • Bug fix
  • Documentation update
  • Refactoring
  • Security patch
  • UI/UX improvement

Description

This PR will provide a route to validate the client config file. A comprehensive, manual one for developers, and another one that automatically redirects users in case their config is incorrectly setup or URLs are unreachable.
It will also update the client dependencies. Related to issue #658.

Testing

Manual testing for now.

Impact

  • Zod dependency.
  • Once complete, the website access flow will be different if your config is wrong.
  • LayoutPublic is parameterized with a possible Breakpoint instead of always being 'xs'.
  • Additional route of '/verify'

Additional Information

None.

Checklist

  • My code adheres to the coding and style guidelines of the project.
  • I have added tests for all the new code and any changes made to
    existing code.
  • I have made corresponding changes to the documentation.

Copy link
Contributor

@prasadtalasila prasadtalasila left a comment

Choose a reason for hiding this comment

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

@atomicgamedeveloper thanks for the PR. Please check the comments.

client/src/route/auth/VerifyConfig.tsx Show resolved Hide resolved
client/src/page/LayoutPublic.tsx Show resolved Hide resolved
atomicgamedeveloper and others added 9 commits November 29, 2024 18:01
)

- Refactors the classes interacting with gitlab so that
  the exploration of existing digital assets in a gitlab
  repository are possible
- Adds library page which allows to browse existing
  assets in a gitlab repository and select them to
  create new digital twin
- Adds feature to create digital twins from clean slate
- Adds ability to create composite, fleet and
  hierarchical digital twins

---------
Co-authored-by: vanessa <[email protected]>
…on#1102)

  - Updates the configuration files of libms in docker containers
  - Updates the documentation to reflect the change.

---------
Co-authored-by: prasadtalasila <[email protected]>
@prasadtalasila prasadtalasila added this to the Release v0.7.0 milestone Dec 13, 2024
client/package.json Outdated Show resolved Hide resolved
Copy link
Contributor

@prasadtalasila prasadtalasila left a comment

Choose a reason for hiding this comment

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

@atomicgamedeveloper Something has gone wrong with the rebase. Also please check the comments.

client/src/route/auth/VerifyConfig.tsx Show resolved Hide resolved
client/package.json Show resolved Hide resolved
client/src/route/auth/ConfigItems.tsx Show resolved Hide resolved
client/src/route/auth/ConfigItems.tsx Show resolved Hide resolved
@prasadtalasila prasadtalasila marked this pull request as ready for review December 17, 2024 15:47
@prasadtalasila
Copy link
Contributor

@atomicgamedeveloper The codeclimate PR integration is broken. Please follow the temporary fix suggested in #1113. Please close this PR and open a new PR from your feature/distributed-demo branch.

@prasadtalasila
Copy link
Contributor

@atomicgamedeveloper Other suggestions.

  1. The auth logic need not have code for config validation. How about having src/route/config for /verify route? Is there any disadvantage to it? If you think that config shown to the user must be different from the one shown to the developer, you can always have config/user and config/developer (instead of config/verify).
  2. Please change the src/build/env.js to have the following content.
    if (typeof window !== 'undefined') {
      window.env = {
        REACT_APP_ENVIRONMENT: 'dev',
        REACT_APP_URL: 'http://localhost:4000/',
        REACT_APP_URL_BASENAME: '',
        REACT_APP_URL_DTLINK: '/lab',
        REACT_APP_URL_LIBLINK: '',
        REACT_APP_WORKBENCHLINK_VNCDESKTOP: '/tools/vnc/?password=vncpassword',
        REACT_APP_WORKBENCHLINK_VSCODE: '/tools/vscode/',
        REACT_APP_WORKBENCHLINK_JUPYTERLAB: '/lab',
        REACT_APP_WORKBENCHLINK_JUPYTERNOTEBOOK: '',
        REACT_APP_WORKBENCHLINK_LIBRARY_PREVIEW: '/preview/library',
        REACT_APP_WORKBENCHLINK_DT_PREVIEW: '/preview/digitaltwins',
    
        REACT_APP_CLIENT_ID: '1be55736756190b3ace4c2c4fb19bde386d1dcc748d20b47ea8cfb5935b8446c',
        REACT_APP_AUTH_AUTHORITY: 'https://gitlab.com/',
        REACT_APP_REDIRECT_URI: 'http://localhost:4000/Library',
        REACT_APP_LOGOUT_REDIRECT_URI: 'http://localhost:4000',
        REACT_APP_GITLAB_SCOPES: 'openid profile read_user read_repository api',
      };
    };
  3. There is an infinite loop in the config validation. It seems that the config validation code is making a lot of requests to the server. You can check with the following commands.
    1. yarn clean, yarn install, yarn build, yarn start
    2. Open browser and visit localhost:4000
    3. See the terminal log
      Please check for this problem for the developer verify route as well.
  4. We discussed about showing a simpler message to users. The message to be shown is:
    Invalid Application Configuration. Please contact the administrator of your DTaaS installation.
  5. When the https://gitlab.com URL is used, the login button is shown but the browser client is making lot of requests in the background. Please check it using Web developer Tools.
  6. The configuration logic is embedded in . The config validation happens only once while the auth validation happens every time for change in a route. So the config logic needs to be brought out of auth route. How about replacing <LayoutPublic> with new <Config> react element for wrapping <SignIn />. If you want, the existing config validation code could be moved from auth route to signin route. In any case, the code structuring has to be such that:
    1. Configuration validation is done only on the signin page.
    2. Two routes: config/user and config/developer are wrapped using <LayoutPublic>
  7. For getValidationResults() method, please see if you can use an Promise.all with array or record iterable.

PS: I haven't checked the tests yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Validate react website configuration
4 participants