redis / lettuce

Advanced Java Redis client for thread-safe sync, async, and reactive usage. Supports Cluster, Sentinel, Pipelining, and codecs.
https://lettuce.io
MIT License
5.39k stars 970 forks source link

`SCAN` not working when using a read-only user #2907

Closed BalmungSan closed 3 months ago

BalmungSan commented 3 months ago

Bug Report

Environment

Current Behavior

When running a SCAN against the database using the read-only user, we receive the following exception:

io.lettuce.core.RedisCommandExcecutionException: ERR internal error

Note: other read-only commands like ZRANGE work as expected. Also, using a user whose ACL is on ~* +@all, the SCAN also works as expected.

Stack trace ```java io.lettuce.core.RedisCommandExecutionException: ERR internal error io.lettuce.core.internal.ExceptionFactory.createExecutionException(ExceptionFactory.java:147) io.lettuce.core.internal.ExceptionFactory.createExecutionException(ExceptionFactory.java:116) io.lettuce.core.protocol.AsyncCommand.completeResult(AsyncCommand.java:120) io.lettuce.core.protocol.AsyncCommand.complete(AsyncCommand.java:111) io.lettuce.core.protocol.CommandHandler.complete(CommandHandler.java:745) io.lettuce.core.protocol.CommandHandler.decode(CommandHandler.java:680) io.lettuce.core.protocol.CommandHandler.channelRead(CommandHandler.java:597) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:442) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420) io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412) io.netty.handler.ssl.SslHandler.unwrap(SslHandler.java:1475) io.netty.handler.ssl.SslHandler.decodeJdkCompatible(SslHandler.java:1338) io.netty.handler.ssl.SslHandler.decode(SslHandler.java:1387) io.netty.handler.codec.ByteToMessageDecoder.decodeRemovalReentryProtection(ByteToMessageDecoder.java:529) io.netty.handler.codec.ByteToMessageDecoder.callDecode(ByteToMessageDecoder.java:468) io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:290) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:444) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420) io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412) io.netty.channel.DefaultChannelPipeline$HeadContext.channelRead(DefaultChannelPipeline.java:1410) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:440) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420) io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:919) io.netty.incubator.channel.uring.AbstractIOUringStreamChannel$IOUringStreamUnsafe.readComplete0(AbstractIOUringStreamChannel.java:299) io.netty.incubator.channel.uring.AbstractIOUringChannel$AbstractUringUnsafe.readComplete(AbstractIOUringChannel.java:482) io.netty.incubator.channel.uring.IOUringEventLoop.handleRead(IOUringEventLoop.java:315) io.netty.incubator.channel.uring.IOUringEventLoop.handle(IOUringEventLoop.java:281) io.netty.incubator.channel.uring.UserData.decode(UserData.java:30) io.netty.incubator.channel.uring.IOUringCompletionQueue.process(IOUringCompletionQueue.java:90) io.netty.incubator.channel.uring.IOUringEventLoop.run(IOUringEventLoop.java:222) io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:997) io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74) io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30) java.lang.Thread.run(Thread.java:1583) delay @ dev.profunktor.redis4cats.effect.FutureLift$$anon$1.lift(FutureLift.scala:50) fromCompletionStage @ dev.profunktor.redis4cats.effect.FutureLift$$anon$1.lift(FutureLift.scala:50) fromCompletableFuture @ work.app.IAMRedisCredentialsProvider$$anon$1.generateIAMToken$$anonfun$2$$anonfun$2(IAMRedisCredentialsProvider.scala:97) map @ dev.profunktor.redis4cats.BaseRedis.scan$$anonfun$4(redis.scala:546) delay @ dev.profunktor.redis4cats.effect.FutureLift$$anon$1.delay(FutureLift.scala:43) flatMap @ dev.profunktor.redis4cats.BaseRedis.scan(redis.scala:546) ```

Expected behavior/code

We would expect to be able to run a SCAN with a read-only user.

Possible Solution

I am not totally sure if this is related or not, but the first Lettuce log when we initiate the SCAN is the following:

DEBUG io.lettuce.core.cluster.PooledClusterConnectionProvider -- getConnection(WRITE, "A HEX STRING")

This makes me think that the problem is that somewhere in the internal implementation details of Lettuce, SCAN is treated as a write command, rather than a read-only one. Which, ultimately leads to the issue we are seeing.

tishun commented 3 months ago

Hey @BalmungSan ,

the code where this error is thrown parses the result from the server, so it is the server that throws "ERR internal error" and not the driver.

Are you using Amazon ElastiCache? Have you tried to execute this command by using some other client, e.g. redis-cli or Redis Insight?

BalmungSan commented 3 months ago

Hi @tishun

the code where this error is thrown parses the result from the server, so it is the server that throws "ERR internal error" and not the driver.

Yes, I understand that.

My gut feeling is that the reason Redis is failing is because Lettuce is trying to use a write connection but the user only has read-only permissions. However, as I said before, this could be completely off. But, considering that the same command works when using a user with all permissions, it feels only natural to assume that the issue is related to permissions. Yet, when checking the official Redis docs for SCAN it only mentions the following ACL categories: @keyspace, @read, @slow, all of which are assigned to the read-only user.

Are you using Amazon ElastiCache?

Yes, that was mentioned in the report.

Have you tried to execute this command by using some other client, e.g. redis-cli or Redis Insight?

No, but due to IAM authentication (which I forgot to mention before, apologies) we really can't test the command using the same user. I will try to test today with another read-only user but without IAM and using redis-cli.

BalmungSan commented 3 months ago

@tishun okay, we just tested using redis-cli and found the same issue. Moving the bug report to Redis itself: https://github.com/redis/redis/issues/13402 Apologies for the noise.

tishun commented 3 months ago

Thanks for verifying that @BalmungSan