Skip to content
This repository has been archived by the owner on Jun 10, 2024. It is now read-only.

login redirection - logic error, and sensitivity to / in URL #41

Closed
planetf1 opened this issue Jan 8, 2021 · 16 comments
Closed

login redirection - logic error, and sensitivity to / in URL #41

planetf1 opened this issue Jan 8, 2021 · 16 comments

Comments

@planetf1
Copy link
Member

planetf1 commented Jan 8, 2021

Running our 'coco pharma' lab helm chart, deployment goes well, and the presentation server pod is started as expected

However login fails

$ kubectl logs lab-odpi-egeria-lab-presentation-78cb55bf9f-wpnbw [15:33:26]

[email protected] prod /home/node/egeria-react-ui/cra-server
NODE_ENV=production node index.js
getServerInfoFromEnv() 1
getServerInfoFromEnv() 2
Found server name coco
Warning: connect.session() MemoryStore is not
designed for a production environment, as it will leak
memory, and will not scale past a single process.
Server listening on port: 8091
segment1 user
/user
undefined
segment1 favicon.ico

Screenshot 2021-01-08 at 15 32 59

@planetf1
Copy link
Member Author

planetf1 commented Jan 8, 2021

This could be an issue in how the image is built.

Any ideas @davidradl

To clarify- the build process currently just does

  • npm install / run build in client, then server
  • in server runs 'npm run prod' in the cra-server directory

See the 'Dockerfile' for what is RUN, and .github/workflows/node-build.yml for what is built.

@planetf1
Copy link
Member Author

planetf1 commented Jan 8, 2021

This occurs when using the base URl ie https://71b52643-eu-gb.lb.appdomain.cloud:8091 which now redirects to a login page.

When entering https://71b52643-eu-gb.lb.appdomain.cloud:8091/coco/login as before, a blank screen is shown both in safari & chrome

However https://71b52643-eu-gb.lb.appdomain.cloud:8091/coco works ok

So overall the behaviour is more sensible, but may be confusing for prior users, also looks like some more changes are needed on the redirect from the other urls

@planetf1
Copy link
Member Author

planetf1 commented Jan 8, 2021

Quick observation - the animation is quite distracting

@planetf1 planetf1 changed the title Create Failed - trying to use new docker image for react UI login redirection - logic error Jan 12, 2021
@planetf1 planetf1 changed the title login redirection - logic error login redirection - logic error, and sensitivity to / in URL Jan 21, 2021
@planetf1
Copy link
Member Author

Discussed with @davidradl via webex

It turns out the issue is, given an organization name 'org'

http://server:8091/
-> Shows full login page
-> A valid user login results in 'Logic Error'
-> We need a 'good' landing page that explains this is invalid - we have no tenant information in context

https://server:8091/org
-> Shows a blank page
-> This should show the login page IMO, the same as .....

https://server:8091/org/
-> Shows the login page
-> Login works fine

https://server:8091/org/login
-> Blank Page
-> optionally could redirect - but not essential. If not should show error

https://server:8091/org/login/
-> Blank Page
-> either redirect or show error as we have enough context for the org to use in the URL

https://server:8091/acme/
-> Shows login page
-> Login works
-> Seems like an error. That page should show an error (no such tenant etc). Also the login shouldn't work (which credentials?). (An error does occur if one then tries to use the UI, but this IMO is too late)

@davidradl
Copy link
Member

@planetf1 please can you verify on the current build; it was looking in the wrong place for its minified resources.

@planetf1
Copy link
Member Author

Testing with the latest build in dev mode gives me

  • npm start takes me to http://localhost:3000/
  • attempt to login as garygeeke/admin - fails with Logic error as before. (I expect the login to be invalid, but we need to correct how that landing page is rendered & whether there even is a login option)
  • Trying http://localhost:3000/coco - works, without the trailing slash - so this is an improvement

So the / is working in this env, but we still have a problem with the root page
Not tested prod yet. Will update when I have.

@planetf1
Copy link
Member Author

I see the same behaviour in the container environment.

I also note that going to https:///acme/

renders a page and then a logic error is produced.

I see two options

Either
a) Land on some kind of error/not here page
b) Just return the same error as if userid/password is incorrect -- ie not admitting to the fact that endpoint is valid or not..?

davidradl referenced this issue in davidradl/egeria-react-ui Feb 23, 2021
Signed-off-by: David Radley <[email protected]>
davidradl referenced this issue in davidradl/egeria-react-ui Feb 23, 2021
Signed-off-by: David Radley <[email protected]>
davidradl added a commit that referenced this issue Feb 23, 2021
@davidradl
Copy link
Member

@planetf1 Please confirm this is now working as expected for you. There has been more than one change in this area to address the issues.

@planetf1
Copy link
Member Author

@davidradl Thanks. Not going to be able to test immediately, so I will assign to myself to verify

@planetf1 planetf1 assigned planetf1 and unassigned davidradl Feb 24, 2021
davidradl referenced this issue in davidradl/egeria-react-ui Feb 25, 2021
@planetf1
Copy link
Member Author

planetf1 commented Mar 3, 2021

I retested this with 2.7 and noticed:

  • If I go straight to the base URL I get a page prompting for server name (see UI docs and panel - terminology re: server name #90)
  • Entering coco correctly redirects me to the correct login URL
  • Entering a bad server name still gives me a login page, though attempting login results in The URL is not valid. You will be asked to put in a valid Server Name. - is this as expected? I had thought the initial url like https://localhost:8091/banana might immediately produce the error
  • On the initial page prompting for server name, I can click to login, however the natural inclination is to enter the server name and hit ENTER - this does not work

@planetf1 planetf1 assigned davidradl and unassigned planetf1 Mar 8, 2021
@davidradl
Copy link
Member

davidradl commented Apr 22, 2021

@planetf1
If I go straight to the base URL I get a page prompting for server name (see #90) working as expected
Entering coco correctly redirects me to the correct login URL working as expected
Entering a bad server name still gives me a login page, though attempting login results in The URL is not valid. You will be asked to put in a valid Server Name. - is this as expected? I had thought the initial url like https://localhost:8091/banana might immediately produce the error I can see this could be improved, though I am not giving this any priority at this time; unless the community thinks this should be looked at with some urgency.
On the initial page prompting for server name, I can click to login, however the natural inclination is to enter the server name and hit ENTER - this does not work I think an explicit login button is acceptable here

I suggest closing this issue as the reported bug is fixed and raising new issues for your remaining concerns

@planetf1
Copy link
Member Author

planetf1 commented Jun 1, 2021

In this release I note that a url like https://server:8091/coco no longer appears to work for login as it redirects to a request for server.
Error, forbidden URL. Please supply a valid Server Name in the URL.

However https://server:8091/coco/login does still work

@planetf1 planetf1 mentioned this issue Jun 1, 2021
26 tasks
@github-actions
Copy link

github-actions bot commented Aug 1, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 20 days if no further activity occurs. Thank you for your contributions.

@planetf1
Copy link
Member Author

https://host:8091/ redirects to page asking for 'server'
Entering 'coco' in this fails and reloads the page prompting for server
Entering https://host:8091/coco works fine
Entering https://host:8091/banana results in Error, forbidden URL. Please supply a valid Server Name in the URL.

@github-actions
Copy link

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 20 days if no further activity occurs. Thank you for your contributions.

@github-actions
Copy link

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 20 days if no further activity occurs. Thank you for your contributions.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants