feature request #7993: Display "Access Denied" dialog in case the remoting service responds with status code = 4xx - EDIT - EDIT Project Management

Actions

Copy link

#1

Updated by Andreas Kohlbecker over 5 years ago

Due date set to 01/10/2019
Start date changed from 01/14/2019 to 01/10/2019
Follows feature request #7972: explicitly allow disallow access to HTTP Invoker endpoints (/remoting/**) added

Actions

Copy link

#2

Updated by Andreas Müller over 5 years ago

Due date deleted (~~01/10/2019~~)
Assignee changed from Andreas Müller to Katja Luther

Actions

Copy link

#3

Updated by Andreas Müller over 5 years ago

Priority changed from New to Highest
Severity changed from normal to major

This is urgent and should definetely be done in 5.5

Actions

Copy link

#4

Updated by Andreas Müller over 5 years ago

By the way, currently the login is successful but only then the exception is thrown I guess because I do not have admin rights anymore after the login (I changed from admin to a user without ROLE_REMOTING rights). Also the Admin menu was removed.

So I think the ROLE_REMOTING rights should already be checked during authentication and authentication should not be successful if the role is not available.

Actions

Copy link

#5

Updated by Andreas Kohlbecker over 5 years ago

How should this be possible?

Successful authentication is the required step for having a "user" in the security context. Otherwise the authentication can not be tested for a GrantedAuthority. We do in fact not test for the role ROLE_REMOTING directly. All this done by Spring Security.

Well, the behavior of Spring Security can be changed if needed, but why should we?

Actions

Copy link

#6

Updated by Andreas Kohlbecker over 5 years ago

So I think the ROLE_REMOTING rights should already be checked during authentication and authentication should not be successful if the role is not available.

Do you suggest to check the role from within the Taxeditor? If you do this, you make assumptions on the webservice security configuration which might be correct by now, but what if the security configuration changes, are you shure that the role ROLE_REMOTIN will be the only criterion in future.

To do this correctly you would need to interpret the security configuration:

.anonymous().disable()
             .antMatcher("/remoting/**")
                 .authorizeRequests().anyRequest().access("hasAnyRole('ROLE_ADMIN', 'ROLE_REMOTING')")
                 .and()
             .csrf().disable()
             .httpBasic();

Actions

Copy link

#7

Updated by Andreas Müller over 5 years ago

I do not really care about technical details of the implementation. There are multiple possiblities. E.g. we could add a second authentication method to service layer authenticateAndCheckRemotingRights(username, ...) or something similar which first checks if the given username has the remoting rights at all and then authenticates. I can't imagine that this is not possible.
Maybe it is also possible to revert to the old authentification if after authentification checking for ROLE_REMOTING (or ROLE_ADMIN) was not successful. As I am not so deep into authentification details I don't know if this is possible.

I only wanted to say that the current implementation is not intuitive and very unexpected behaviour. If I try to login into something where I have absolutely no rights I wouldn't expect that the login is still successful but I would expect that I end up with the state I am in before login.

Actions

Copy link

#8

Updated by Andreas Müller over 5 years ago

Andreas Kohlbecker wrote:

, but what if the security configuration changes, are you sure that the role ROLE_REMOTIN will be the only criterion in future.

More or less, yes. Why should there be another role which is absolutely required to allow ANY access to the remoting services. But if there will be another criterion, we have to add it.

Actions

Copy link

#9

Updated by Andreas Kohlbecker over 5 years ago

If I try to login into something where I have absolutely no rights I wouldn't expect that the login is still successful

That exactly is the point here. After logging in the server you have access to a couple of webservices, but the access to /remoting/** is denied. You still can use the REST api, etc. This is only irritating from the taxeditor from the view on the whole platform this is 100% ok.

I think the editor should catch the exception and display a message and that's all. The usecase that a user is switching from 'Admin' to an unpriviledges user is quite rare and special. Well, and if this happens to you, you need to login again as 'Admin'

I actually don't see the problem here.

Actions

Copy link

#10

Updated by Andreas Müller about 5 years ago

Andreas Kohlbecker wrote:

If I try to login into something where I have absolutely no rights I wouldn't expect that the login is still successful

That exactly is the point here. After logging in the server you have access to a couple of webservices, but the access to /remoting/** is denied. You still can use the REST api, etc. This is only irritating from the taxeditor from the view on the whole platform this is 100% ok.

We are only talking about the TaxEditor perspective here. Sure for other applications this discussion is different/not relevant.

I think the editor should catch the exception and display a message and that's all. The usecase that a user is switching from 'Admin' to an unpriviledges user is quite rare and special. Well, and if this happens to you, you need to login again as 'Admin'

I agree that the usecase is rare (except for testing/debugging) therefore we may implement a simpler solution if the proposed solution is difficult to implement.
So what about a dialog saying that login was successful but no rights exist for the given user to use the TaxEditor. Therefore the user has been logged off again. Please relogin with suffient rights. At the same time the TaxEditor automatically logsoff and reopens the login dialog. This should be easy to implement and easy to understand for the user.

Actions

Copy link

#11

Updated by Andreas Kohlbecker about 5 years ago

So what about a dialog saying that login was successful but no rights exist for the given user to use the TaxEditor.

100% agreed!!!! BTW, this suggestion exactly matches the ticket description ;-) except of the fact that the message presented by the dialog should rather be "You are logged in now but you are not permitted to use the TaxEditor with the data source ${dataSourceName}" instead of a harsh "Access Denied"

The LoginDialog could perform a simple check by attempting to use a service method. The Exception can easily catched in this situation and the LogniDialog can be kept open, so that the user can change the data source or the user account without needing to go into the menu again.

Actions

Copy link

#12

Updated by Katja Luther about 5 years ago

Status changed from New to Resolved
Assignee changed from Katja Luther to Andreas Müller

Now the login dialog is showing up again with a message that the user is logged in but has no rights to see the data with the taxeditor.
The message is shown only partly but the whole messsage is shown in a mouseover text.

please review. (for example with user2 in rem_conf_am and password test123)

Actions

Copy link

#13

Updated by Andreas Müller about 5 years ago

Description updated (diff)

Actions

Copy link

#14

Updated by Andreas Müller about 5 years ago

File picture061-1.png picture061-1.png added

I still get a not nice looking dialog when loging in with no rights:

login : norights
editor version : 5.5.0.201901242349
server : test.e-taxonomy.eu (edit-test) / rem_conf_am
schema version : 5.0.0.0.20180514
os : Windows Server 2012 R2 6.3 amd64
java : 1.8.0_121
org.eclipse.swt.SWTException: Failed to execute runnable (org.springframework.remoting.RemoteAccessException: Could not access HTTP invoker remote service at [http://test.e-taxonomy.eu:80/cdmserver/rem_conf_am/remoting/term.service]; nested exception is org.apache.http.NoHttpResponseException: Did not receive successful HTTP response: status code = 403, status message = [Access is denied])
    at org.eclipse.swt.SWT.error(SWT.java:4533)
    at org.eclipse.swt.SWT.error(SWT.java:4448)
    at org.eclipse.swt.widgets.Synchronizer.runAsyncMessages(Synchronizer.java:185)
    at org.eclipse.swt.widgets.Display.runAsyncMessages(Display.java:4211)
    at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3827)
    at org.eclipse.jface.operation.ModalContext$ModalContextThread.block(ModalContext.java:165)
    at org.eclipse.jface.operation.ModalContext.run(ModalContext.java:369)
    at org.eclipse.jface.dialogs.ProgressMonitorDialog.run(ProgressMonitorDialog.java:481)
    at eu.etaxonomy.taxeditor.store.CdmStoreConnector.start(CdmStoreConnector.java:147)
    at eu.etaxonomy.taxeditor.store.CdmStore.connect(CdmStore.java:261)
    at eu.etaxonomy.taxeditor.store.CdmStore.connect(CdmStore.java:216)
    at eu.etaxonomy.taxeditor.ui.dialog.RemotingLoginDialog.connect(RemotingLoginDialog.java:908)
    at eu.etaxonomy.taxeditor.ui.dialog.RemotingLoginDialog.connectButtonPressed(RemotingLoginDialog.java:486)
    at eu.etaxonomy.taxeditor.ui.dialog.RemotingLoginDialog.access$2(RemotingLoginDialog.java:482)
    at eu.etaxonomy.taxeditor.ui.dialog.RemotingLoginDialog$6.widgetSelected(RemotingLoginDialog.java:365)
    at org.eclipse.swt.widgets.TypedListener.handleEvent(TypedListener.java:249)
    at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:84)
    at org.eclipse.swt.widgets.Display.sendEvent(Display.java:4418)
    at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1079)
    at org.eclipse.swt.widgets.Display.runDeferredEvents(Display.java:4236)
    at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3824)
    at eu.etaxonomy.taxeditor.ui.dialog.RemotingLoginDialog.open(RemotingLoginDialog.java:214)
    at eu.etaxonomy.taxeditor.ui.dialog.RemotingLoginDialog.open(RemotingLoginDialog.java:189)
    at eu.etaxonomy.taxeditor.ui.dialog.RemotingLoginDialog.open(RemotingLoginDialog.java:180)
    at eu.etaxonomy.taxeditor.handler.e4.SwitchUserHandlerE4.execute(SwitchUserHandlerE4.java:37)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:498)
    at org.eclipse.e4.core.internal.di.MethodRequestor.execute(MethodRequestor.java:55)
    at org.eclipse.e4.core.internal.di.InjectorImpl.invokeUsingClass(InjectorImpl.java:282)
    at org.eclipse.e4.core.internal.di.InjectorImpl.invoke(InjectorImpl.java:264)
    at org.eclipse.e4.core.contexts.ContextInjectionFactory.invoke(ContextInjectionFactory.java:132)
    at org.eclipse.e4.core.commands.internal.HandlerServiceHandler.execute(HandlerServiceHandler.java:152)
    at org.eclipse.core.commands.Command.executeWithChecks(Command.java:494)
    at org.eclipse.core.commands.ParameterizedCommand.executeWithChecks(ParameterizedCommand.java:488)
    at org.eclipse.e4.core.commands.internal.HandlerServiceImpl.executeHandler(HandlerServiceImpl.java:210)
    at org.eclipse.e4.ui.workbench.renderers.swt.HandledContributionItem.executeItem(HandledContributionItem.java:433)
    at org.eclipse.e4.ui.workbench.renderers.swt.AbstractContributionItem.handleWidgetSelection(AbstractContributionItem.java:454)
    at org.eclipse.e4.ui.workbench.renderers.swt.AbstractContributionItem$3.handleEvent(AbstractContributionItem.java:482)
    at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:84)
    at org.eclipse.swt.widgets.Display.sendEvent(Display.java:4418)
    at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1079)
    at org.eclipse.swt.widgets.Display.runDeferredEvents(Display.java:4236)
    at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3824)
    at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine$4.run(PartRenderingEngine.java:1121)
    at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:336)
    at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.run(PartRenderingEngine.java:1022)
    at org.eclipse.e4.ui.internal.workbench.E4Workbench.createAndRunUI(E4Workbench.java:150)
    at org.eclipse.ui.internal.Workbench$5.run(Workbench.java:693)
    at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:336)
    at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:610)
    at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:148)
    at eu.etaxonomy.taxeditor.Application.start(Application.java:24)
    at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:196)
    at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:134)
    at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:104)
    at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:388)
    at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:243)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:498)
    at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:673)
    at org.eclipse.equinox.launcher.Main.basicRun(Main.java:610)
    at org.eclipse.equinox.launcher.Main.run(Main.java:1519)
Caused by: org.springframework.remoting.RemoteAccessException: Could not access HTTP invoker remote service at [http://test.e-taxonomy.eu:80/cdmserver/rem_conf_am/remoting/term.service]; nested exception is org.apache.http.NoHttpResponseException: Did not receive successful HTTP response: status code = 403, status message = [Access is denied]
    at org.springframework.remoting.httpinvoker.HttpInvokerClientInterceptor.convertHttpInvokerAccessException(HttpInvokerClientInterceptor.java:216)
    at org.springframework.remoting.httpinvoker.HttpInvokerClientInterceptor.invoke(HttpInvokerClientInterceptor.java:147)
    at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:179)
    at org.springframework.aop.framework.JdkDynamicAopProxy.invoke(JdkDynamicAopProxy.java:208)
    at com.sun.proxy.$Proxy46.findWithoutFlush(Unknown Source)
    at eu.etaxonomy.cdm.api.cache.CdmServiceCacher.findByUuid(CdmServiceCacher.java:93)
    at eu.etaxonomy.cdm.api.cache.CdmCacher.load(CdmCacher.java:134)
    at eu.etaxonomy.cdm.model.common.DefinedTermBase.getTermByClassAndUUID_aroundBody20(DefinedTermBase.java:540)
    at eu.etaxonomy.cdm.model.common.DefinedTermBase.getTermByClassAndUUID(DefinedTermBase.java:1)
    at eu.etaxonomy.cdm.model.name.Rank.getTermByUuid(Rank.java:230)
    at eu.etaxonomy.cdm.model.name.Rank.initDefaultTerms_aroundBody174(Rank.java:1016)
    at eu.etaxonomy.cdm.model.name.Rank.initDefaultTerms(Rank.java:1)
    at eu.etaxonomy.taxeditor.store.CdmStoreConnector$1$2.run(CdmStoreConnector.java:211)
    at org.eclipse.swt.widgets.RunnableLock.run(RunnableLock.java:35)
    at org.eclipse.swt.widgets.Synchronizer.runAsyncMessages(Synchronizer.java:182)
    ... 63 more
Caused by: org.apache.http.NoHttpResponseException: Did not receive successful HTTP response: status code = 403, status message = [Access is denied]
    at org.springframework.remoting.httpinvoker.HttpComponentsHttpInvokerRequestExecutor.validateResponse(HttpComponentsHttpInvokerRequestExecutor.java:357)
    at org.springframework.remoting.httpinvoker.HttpComponentsHttpInvokerRequestExecutor.doExecuteRequest(HttpComponentsHttpInvokerRequestExecutor.java:230)
    at eu.etaxonomy.taxeditor.service.CdmServiceRequestExecutor.doExecuteRequest(CdmServiceRequestExecutor.java:61)
    at org.springframework.remoting.httpinvoker.AbstractHttpInvokerRequestExecutor.executeRequest(AbstractHttpInvokerRequestExecutor.java:138)
    at org.springframework.remoting.httpinvoker.HttpInvokerClientInterceptor.executeRequest(HttpInvokerClientInterceptor.java:194)
    at org.springframework.remoting.httpinvoker.HttpInvokerClientInterceptor.executeRequest(HttpInvokerClientInterceptor.java:176)
    at org.springframework.remoting.httpinvoker.HttpInvokerClientInterceptor.invoke(HttpInvokerClientInterceptor.java:144)
    ... 76 more

Actions

Copy link

#15

Updated by Andreas Müller about 5 years ago

File picture554-1.png picture554-1.png added
Status changed from Resolved to Feedback
Assignee changed from Andreas Müller to Katja Luther

But the login itself seems to work:

User is "norights"

Actions

Copy link

#16

Updated by Katja Luther about 5 years ago

Status changed from Feedback to Resolved
Assignee changed from Katja Luther to Andreas Müller

sorry I missed to commit some of the changes. please review.

Actions

Copy link

#17

Updated by Andreas Müller about 5 years ago

Status changed from Resolved to Feedback
Assignee changed from Andreas Müller to Katja Luther

I still get the same error. Can you please test yourself on nightly.

Actions

Copy link

#18

Updated by Andreas Müller about 5 years ago

I tested a bit further. It looks like feature works if nothing is opened yet before swithing user.

I tested with 1 taxon open and user bulkeditor open => exception (I switched in rem_conf_am) from admin to norights

Actions

Copy link

#19

Updated by Andreas Müller about 5 years ago

Also the message is too long for the message text field. We should change the layout or shorten the message.

Actions

Copy link

#20

Updated by Katja Luther about 5 years ago

Andreas Müller wrote:

Also the message is too long for the message text field. We should change the layout or shorten the message.

You get the whole message as tooltip. But we also can shorten the message

Actions

Copy link

#21

Updated by Andreas Müller about 5 years ago

Katja Luther wrote:

Andreas Müller wrote:

Also the message is too long for the message text field. We should change the layout or shorten the message.

You get the whole message as tooltip. But we also can shorten the message

Such a message should definetly not be truncated. However we could keep a longer message as tooltip to explain a bit more what happened. In future theses tooltips should go into a helpsystem with a help button beside the message.

Actions

Copy link

#22

Updated by Patrick Plitzner about 5 years ago

Related to bug #8042: UI is too restricted for users with right Project Manager added

Actions

Copy link

#23

Updated by Patrick Plitzner about 5 years ago

Description updated (diff)

Actions

Copy link

#24

Updated by Patrick Plitzner about 5 years ago

Description updated (diff)

Actions

Copy link

#25

Updated by Andreas Kohlbecker about 5 years ago

From the report which can be found in #8042 it seems obvious that it is still possible to use the Editor after a successful login with a user which lacks the authority to use the remoting service. A user without permission to use the removing service should not be able to do anything related to data in the editor.

By now is see the following options

once the login dialog knows the user is not permitted to use the remoting service, it should automatically logout the user. The message displayed to the user needs to be different in this case. "Your credentials are valid but you are not permitted access any data." ... something more compact of course
The login dialog will log out the user once the dialog is closed.

Actions

Copy link

#26

Updated by Andreas Müller about 5 years ago

Andreas Kohlbecker wrote:

From the report which can be found in #8042 it seems obvious that it is still possible to use the Editor after a successful login with a user which lacks the authority to use the remoting service. A user without permission to use the removing service should not be able to do anything related to data in the editor.

By now is see the following options

once the login dialog knows the user is not permitted to use the remoting service, it should automatically logout the user. The message displayed to the user needs to be different in this case. "Your credentials are valid but you are not permitted access any data." ... something more compact of course

The login dialog will log out the user once the dialog is closed.

I don't think the report your conclusion is correct. If it were it means that the complete remoting role does not work correctly, which is handled in #7972. I think you misunderstood why there is data to see. I guess the reporter did change login so there was still "old data" visible (which is not really correct, all views should be emptied when changing user, but it is not critical). I tried to login as first user with no rights and I did not get any data.

Actions

Copy link

#27

Updated by Katja Luther about 5 years ago

Status changed from Feedback to Resolved
Assignee changed from Katja Luther to Andreas Müller

the message is adapted and the message part of the login dialog is larger, now.

cleaning all views when changing the user I move to a new ticket.

Actions

Copy link

#28

Updated by Andreas Müller about 5 years ago

File picture875-1.png picture875-1.png added
Status changed from Resolved to Feedback
Assignee changed from Andreas Müller to Katja Luther
% Done changed from 0 to 90

Seems to work now as expected except for the non emptied views. Can you please link to the new ticket and then close this ticket.

Logout as suggested in #7993#note-25 is not needed IMO. When ever I try to access data I get the message

which is ok. Also in theory data can not be available as all data access in TaxEditor works through httpInvoker which is secured.

Actions

Copy link

#29

Updated by Katja Luther about 5 years ago

Status changed from Feedback to Closed
% Done changed from 90 to 100

Actions

Copy link

#30

Updated by Andreas Müller about 5 years ago

Andreas Müller wrote:

Seems to work now as expected except for the non emptied views. Can you please link to the new ticket and then close this ticket.

I think the link is still missing.

Actions

Copy link

#31

Updated by Katja Luther about 5 years ago

the closing of views is implemted now (see last commit). so we do not need a new ticket and close this one

picture061-1.png (20.6 KB) picture061-1.png		Andreas Müller, 01/25/2019 01:54 PM
picture554-1.png (1.03 KB) picture554-1.png		Andreas Müller, 01/25/2019 01:56 PM
picture875-1.png (6.39 KB) picture875-1.png		Andreas Müller, 02/01/2019 01:24 PM

Project

General

Profile

EDIT

Custom queries