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

Can direct media URLs be disabled? #7272

Closed
szilardcsere89 opened this issue Oct 14, 2020 · 25 comments
Closed

Can direct media URLs be disabled? #7272

szilardcsere89 opened this issue Oct 14, 2020 · 25 comments
Milestone

Comments

@szilardcsere89
Copy link
Contributor

Once the users access the URL of a media file it will be available for everyone and I can't take advantage of my private container (Azure Storage).

I mean ~/media/path-to-blob/blob.jpg is a public URL.

Is it possible to disable downloading media files directly with their URLs?

@deanmarcussen
Copy link
Member

Duplicate of #6027

Not currently supported oob @szilardcsere89 however if you want to implement it suggestions for the implementation detail are on the issue.

Here is some limited middleware I use which secures anything under the path ~/media/secure

    public class SecureMediaMiddleware
    {
        private readonly RequestDelegate _next;
        private readonly ILogger _logger;

        private readonly PathString _assetsRequestPath;

        public SecureMediaMiddleware(
            RequestDelegate next,
            IOptions<MediaOptions> mediaOptions,
            IOptions<UserOptions> userOptions,
            ILogger<SecureMediaMiddleware> logger
            )
        {
            _next = next;
            _logger = logger;

            _assetsRequestPath = mediaOptions.Value.AssetsRequestPath;
        }

        /// <summary>
        /// Secures a media path if request url starts with ~/media/secure
        /// </summary>
        /// <param name="context"></param>
        public async Task Invoke(HttpContext context, IAuthorizationService authorizationService)
        {
            var validateAssetsRequestPath = context.Request.Path.StartsWithNormalizedSegments(_assetsRequestPath, StringComparison.OrdinalIgnoreCase, out var subPath);
            if (!validateAssetsRequestPath)
            {
                _logger.LogDebug("Request path {Path} does not match the assets request path {RequestPath}", subPath, _assetsRequestPath);
                await _next(context);
                return;
            }

            if (subPath.StartsWithNormalizedSegments("/secure"))
            {
                _logger.LogDebug("Request path {SubPath} is secure, authorizing", subPath);
                if (!await authorizationService.AuthorizeAsync(context.User, SecureMediaPermissions.ViewSecureMedia))
                {
                    await context.ChallengeAsync();
                    return;
                }
                _logger.LogDebug("View Media authorization successul");
            }

            // One day if they want thumbnails we could also check the query string for a size of 160
            await _next(context);
            return;
        }
    }

You will need to register app.Use<SecureMediaMiddleware>() this in a module which has a dependency on OrchardCore.Admin to get the middleware loaded in the correct order (needs to be after the standard AuthenticationMiddleware but before the OrchardCore.Media.Middleware)

@szilardcsere89
Copy link
Contributor Author

Thanks, it works. The only problem if the browser caches the asset.

@sebastienros
Copy link
Member

We could create a Media Permission module that would create a ViewMedia permission, and check this in the described middleware. Then it could also support folder specific permissions which has been asked too.

Disabling public access would just be about removing ViewMedia from Anonymous.

@sebastienros sebastienros added this to the 1.0.x milestone Oct 15, 2020
@Piedone
Copy link
Member

Piedone commented Jan 11, 2021

Just a nitpick @deanmarcussen: You may consider returning a 404 instead of a challenge so the existence of restricted files remains a secret too (much how GitHub does it too: There's no difference between opening a URL that is genuinely a 404 and one that hides something you're not allowed to see).

@deanmarcussen
Copy link
Member

Good advise @Piedone

For truly secret files you would prefer that.

This snippet is from a paywall, where it actually lets thumbnails through up to a certain size and looks for membership for media of a higher quality

@Piedone
Copy link
Member

Piedone commented Jan 11, 2021

Ah I see, then it's fine of course.

@intellimedhu
Copy link

@deanmarcussen
Not working anymore. Middleware is ignored when requesting static file directly.

@MikeKry
Copy link
Contributor

MikeKry commented Oct 18, 2021

Can we move media cache to non-public folder, e.g. AppData as mentioned in #10375 ?

@deanmarcussen
Copy link
Member

Can we move media cache to non-public folder, e.g. AppData as mentioned in #10375 ?

Configurable via options, and documented. Move it where you want to

@MikeKry
Copy link
Contributor

MikeKry commented Oct 18, 2021

@deanmarcussen

I am sorry, but I can not find it anywhere in docs. When I searched it in code, I can not see any configurable part (if I do not want to replace a lot of classes)

As it is defined as static string here:
image

and used directly from azure module:
image

I did not find is-cache definition though.

It should solve the issue with direct access to files in middleware mentioned above.

@sebastienros
Copy link
Member

@MikeKry I don't see how it could not be public. What would be the purpose?

@MikeKry
Copy link
Contributor

MikeKry commented Oct 22, 2021

@sebastienros

If I want to hide media files with custom middleware - e.g. the one mentioned above - then I can do it only for routes leading to CMS app (e.g. /media/secure/image.png).

But there is a problem, that image is stored also in cache, e.g. "ms-cache/Project/secure/image.png" where I can not apply middleware and it can be accessed directly without having any permissions. As this direct access to images is not commonly used from websites, I thought it would not be a problem to move it to private path.

Normally I would do it with staticfileoptions, e.g.
image
But it does not work with oc, I guess because it has its own "UseStaticFiles" definition, and even if it would work, I think it is safer to just not have cache files in public folder.

@sebastienros
Copy link
Member

If I want to hide media files with custom middleware

Maybe don't use Media, it hasn't been designed for this usage. You might be able to use a different folder, and you own file provider middleware. And these wouldn't be cached.

@MikeKry
Copy link
Contributor

MikeKry commented Oct 28, 2021

@sebastienros

Yes, that is also an option and if there is no possibility to move these files, then I guess I will have to create custom implementation. I just thought that it could help a lot of people with some smaller portions of private content, also I do not see how moving these files would cause any problems, but maybe I miss something.

Anyway, thanks for responding and tips.

@sebastienros
Copy link
Member

I also think it would be a useful feature. And most can be copy pasted from the current one and just cut what is not necessary.

Going back a little, is anything preventing you from adding a middleware that would also intercept the caches requests too? Maybe my memory is failing in how everything works ...

@MikeKry
Copy link
Contributor

MikeKry commented Oct 28, 2021

I would need to intercept request before staticfile middleware, which means probably that I would need to define my own UseStaticFiles(StaticFileOptions - OnPrepareResponse), but I tried that and it did not work. I think that is because OC also defines OnPrepareResponse method, so they overwrite each other somewhere in lifecycle.

I might give it another shot, but it seems fragile, as I would need it to be always called after last OC StartupBase (not sure if it can be determined), and it would also need to be after useauthorization() so I would have user context .

@jeffolmstead
Copy link
Contributor

I didn't realize until this conversation that this could be impacting me soon also. I have a "private" folder and don't want the files ending up in ms-cache. Am I safe to say ms-cache purpose is to prevent the constant calling of Azure Blob storage (or other external providers) for the same file (i.e. the code throws it in ms-cache so it can be retrieved locally rather than doing API calls every time)? I believe that is the case as, when I simply store files without using an external media store, they are not in cache. Again, this doesn't solve the problem of securing media that is stored on an external store, but at least might help clarify it for anyone who stumbles on this.

If this is the case another alternative to "securing" the cache could be to override IMediaFileStoreCacheFileProvider AFTER OrchardCore.Media.Azure sets it in it's startup... might be tricky, but if there is a project that has a dependency on Orchard.Core.Media.Azure and adds it's own IMediaFileStoreCacheFileProvider then it should override. This is a complete hack as it only works to override Orchard.Core.Media.Azure and not other providers (unless a way can be found to guarantee the project load last at which I would suspect it could work universally).

As mentioned, it seems the best approach is to find an option to have ms-cache folder located in App_Data rather than wwwroot. Is there any negative to doing it that way? Are there actual reasons it is beneficial to have ms-cache available to the public? I don't see where anyone directly / intentionally would browse to files via the ms-cache but again, there might be reasons I am unaware of...

@deanmarcussen
Copy link
Member

I didn’t realize you were talking about the blob storage provider when you raised this @MikeKry .

If you want to disable access to this through the host app I think you should just be able to remove the call to UseStaticFiles in the host Startup.

@alandillon
Copy link

I am using the azure blob storage and I really did not want users to be able to easily access the uploaded data through the CMS url.

I disabled the media folder by putting a folder at that route in iis.

image

And then I put a web.config in that folder with the following:

<configuration>
    <system.web>
        <authorization>
           <deny users="*"/>
        </authorization>
    </system.web>
</configuration>

And TADA

image

@MikeKry
Copy link
Contributor

MikeKry commented Jan 14, 2022

@alandillon

That is an option I ended up doing also, but it is not something I would like to use on every project

@szilardcsere89
Copy link
Contributor Author

I agree. I don't think it's a good solution

@mariojsnunes
Copy link
Contributor

mariojsnunes commented Jan 17, 2022

Found this workaround, seems to work well...

Edit:
Nice blogpost about this... https://dev.to/j_sakamoto/how-can-i-protect-static-files-with-authorization-on-asp-net-core-4l0o

cms.UseStaticFiles(new StaticFileOptions()
{
    ContentTypeProvider = provider
    OnPrepareResponse = cms =>
    {
        if (cms.Context.Request.Path.Value.StartsWith("/ms-cache/") || cms.Context.Request.Path.Value.StartsWith("/is-cache/"))
        {
            cms.Context.Response.ContentLength = 0;
            cms.Context.Response.Body = Stream.Null;
            foreach (var header in cms.Context.Response.Headers)
            {
                cms.Context.Response.Headers.Remove(header.Key);
            }
            cms.Context.Response.StatusCode = StatusCodes.Status401Unauthorized;
            return;
        }
    }
});

What other impacts this code would cause? Is there a reason this shouldn't be done?

@deanmarcussen
Copy link
Member

Why don't you just remove the call to cms.UseStaticFiles().

It's not necesary unless you plan to serve static files from the host sites wwwroot folder. (many static files would come from modules / themes).

Or if you specifically do want to store some static files in the hosts wwwroot, make sub folder in it called public, and register the static file options to serve from that folder only.

@mariojsnunes what you have done is fine.

@sebastienros
Copy link
Member

Custom middleware registration will work too.

@mariojsnunes
Copy link
Contributor

@deanmarcussen yeah I confirm that solution works and is way cleaner.
My project setup uses an Angular SPA, I'll post below the full configure method for future reference:

public void Configure(IApplicationBuilder app, IWebHostEnvironment env)
{
    app.Use(async (context, next) =>
    {
        context.Response.Headers.Add("X-Frame-Options", "SAMEORIGIN");
        context.Response.Headers.Add("X-Xss-Protection", "1; mode=block");
        context.Response.Headers.Add("X-Permitted-Cross-Domain-Policies", new StringValues("none"));
        context.Response.Headers.Add("X-Content-Type-Options", "nosniff");

        await next();
    });

    app.UseResponseCompression();

    // add .webmanifest MIME-type support
    FileExtensionContentTypeProvider contentTypeProvider = new FileExtensionContentTypeProvider();
    contentTypeProvider.Mappings[".webmanifest"] = "application/manifest+json";

    app.UseSpaStaticFiles(new StaticFileOptions()
    {
        ContentTypeProvider = contentTypeProvider
    });

    // Host OrchardCore in /cms subfolder
    app.Map(new PathString("/cms"), cms =>
    {
        if (env.IsDevelopment())
        {
            app.UseDeveloperExceptionPage();
        }

        if (env.IsProduction() || env.IsStaging())
        {
            app.UseExceptionHandler("/error");
            app.UseHsts();
        }

        cms.UseStaticFiles(new StaticFileOptions()
        {
            ContentTypeProvider = contentTypeProvider,
            FileProvider = new PhysicalFileProvider(Path.Combine(env.ContentRootPath, "wwwroot/Assets")),
            RequestPath = new PathString("/Assets")
        });

        cms.UseOrchardCore(c => c.UseSerilogTenantNameLogging());
    });

    app.UseSpa(spa =>
    {
        spa.Options.SourcePath = "ClientApp";
    });
}

@sebastienros sebastienros modified the milestones: 1.x, 1.3 Mar 9, 2022
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

No branches or pull requests

9 participants