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: sas9 public access denied handling #755

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Conversation

medjedovicm
Copy link
Member

Issue

Fixes #150

Intent

Covers the SAS9 Public Access Denied scenario. 2 places touched:

  1. login() - User is not logged in. The login function is called with Public Account credentials. The adapter will return an object that contains an error message with error details and loggedIn is false. (image 1.)
  2. request() - User with Public Account is already logged in within current browser. App loads, request function is called. The adapter will throw an error with details inside. (image 2.)

image 1.
public-access-on-login

image 2.
public-access-on-request

Checks

No PR (that involves a non-trivial code change) should be merged, unless all items below are confirmed! If an urgent fix is needed - use a tar file.

  • All sasjs-cli unit tests are passing (npm test).
  • (CI Runs this) All sasjs-tests are passing. If you want to run it manually (instructions available here).
  • Data Controller builds and is functional on both SAS 9 and Viya

@github-actions
Copy link

github-actions bot commented Jul 12, 2023

Coverage report

Total coverage

Status Category Percentage Covered / Total
🟡 Statements 62.72% 2027/3232
🔴 Branches 44.67% 738/1652
🔴 Functions 53.56% 301/562
🟡 Lines 73.99% (+2.25% 🔼) 3283/4437

Status of coverage: 🟢 - ok, 🟡 - slightly more than threshold, 🔴 - under the threshold

Show files with reduced coverage 🔻

Reduced coverage

Status Filename Statements Branches Functions Lines
🟢 src/job-execution/WebJobExecutor.ts 16.07% 2.6% 10.53% 100% (+72.85% 🔼)

Status of coverage: 🟢 - ok, 🟡 - slightly more than threshold, 🔴 - under the threshold

Report generated by 🧪jest coverage report action from 3556eb3

@YuryShkoda
Copy link
Member

@medjedovicm please a test cases for both scenarios

@saadjutt01 saadjutt01 removed their request for review October 27, 2024 20:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Public access denied error is not handled when trying to log in
2 participants