search

MDSplus / mdsplus

The MDSplus data management system
https://mdsplus.org/
Other
75 stars 44 forks source link

alpha versions fail to raise MDSconnection issue #2750

Closed smithsp closed 3 months ago

smithsp commented 7 months ago

Affiliation GA

Version(s) Affected Server on alpha-7-140-72 with clients at 7.7.15 or 7-140-71

Platform The server is on RHEL8. The clients are on linux and osx systems.

Describe the bug Previously (like with the previous server version on atlas) when a connection was denied, then MDSplus would raise a Connection error.

To Reproduce In python:

import MDSplus
c=MDSplus.Connection('atlas.gat.com:8000')  # <------ This doesn't raise an error any more when denied connection.

Expected behavior Expected:

>>> import MDSplus
>>> c=MDSplus.Connection('mdstest.gat.com:8000')
Error in connect: Access denied
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/fusion/projects/codes/atom/omfit_omega_v3.x/atom_git/mambaforge_stable/lib/python3.7/site-packages/MDSplus/connection.py", line 142, in __init__
    raise MdsIpException("Error connecting to %s" % (hostspec,))
MDSplus.connection.MdsIpException: %MDSPLUS-E-Unknown, Error connecting to mdstest.gat.com:8000

This error isn't being thrown with the latest server versions.

Impact OMFIT relies on the exception being thrown to know to tunnel through our bastion host. We have codes in OMFIT that run off-site that need to access MDSplus and are not white listed, so they tunnel through the bastion host.

mwinkel-dev commented 7 months ago

Hi @smithsp -- The "previous server version on Atlas" was stable_7.96.9, correct? If so, that is from 7-Feb-2020.

And yes, there have been changes to connection behavior in the following years. For example, PR #2288 altered some aspects of connections.

Regardless, alpha_7.140.72 should raise an error. I will now investigate what has changed in the past ~4 years.

smithsp commented 7 months ago

@mwinkel-dev I don't mean stable_7.96.9. I mean alpha-7.139-59.

mwinkel-dev commented 7 months ago

Hi @smithsp -- Thanks for the clarification! That news greatly simplifies the troubleshooting.

smithsp commented 7 months ago

Also the bug is not on the ga_atlas build.

mwinkel-dev commented 7 months ago

Hi @smithsp -- Both facts are consistent, which is also good news. In a few minutes, I should be able to reproduce the problem.

mwinkel-dev commented 7 months ago

I have just reproduced the problem with current alpha.

mwinkel-dev commented 7 months ago

Root cause was an incomplete fix for Issue #2652. (Note that the issue was fixed after alpha_7.139.59.)

That bug involved a status variable that was overloaded and thus failed (i.e., allowed unauthorized clients to connect).

The fix for that bug was to eliminate the overloading by using two variables: status and auth_status. Unfortunately, the auth_status wasn't used to set the flag in the message header that was returned to the client.

Submitting a two line PR to complete the fix.

mwinkel-dev commented 6 months ago

Note that the PR #2752 fix handles the scenario when a client successfully establishes a network connection but fails the user authorization check. It returns a failure status to the client so that the client can raise an exception.

The existing code already raises exceptions on these related scenarios: