question: is there a reason we don't log the actual response?

[mrkagelui]

I think, same as the response code, we can also add the actual response to the context and log it, is there any concern/reason we are not doing it?

if not, please let me PR in 😄

[ardan-bkennedy]

You want to log the body of the response?

[mrkagelui]

yeah, is it a bad idea?

[mrkagelui]

I think it's rather important, no?

[ardan-bkennedy]

I think so. Outside of the extra heavy logging, there are potential security risks. I wouldn’t do it.

[mrkagelui]

hmm, heavy meaning disk IO? why security risk? could you give an example?

[mrkagelui]

do you mean some information should only be known by clients, not log viewers like devs?

[ardan-bkennedy]

It'a a lot of extra IO and memory usage for sure.

Logs can be leaked and are rarely secured. You have to be really careful what you put in them. Just putting settings like AWS keys can be a nightmare when the logs fall into the wrong hands.

Each application is different but focus the logs for sure on debugging but assume the logs are readable by everyone.

[mrkagelui]

I see, hmm, great advice, thank you!

[mrkagelui]

Sorry one more question before I go, if it's a requirement by the org, and extra work is done to audit the log etc to ensure the absence of sensitive information, within this framework it'll have to be put in the context right?

[ardan-bkennedy]

I would so you can process this in the logging middleware. I think that's the best place for it. You can't log in the web.Respond function since that is located in the foundation layer.

It's really tricky not to make a mistake. I would rather you write this information to the DB and secure access to it.

You still need to find a place to perform that action, maybe a new middleware function.

[mrkagelui]

That's an interesting advice, thank you, truly appreciated. Let me try to find some horror stories about log leak and scare the s*it out of my teammates into doing the Db way.

[ardan-bkennedy]

You don't have to search hard.

https://www.hindawi.com/journals/scn/2021/6615899/ https://medium.com/@joecrobak/seven-best-practices-for-keeping-sensitive-data-out-of-logs-3d7bbd12904 https://resources.infosecinstitute.com/topic/android-hacking-security-part-4-exploiting-unintended-data-leakage-side-channel-data-leakage/

[mrkagelui]

Sorry for keeping coming back, I've seen another way of doing this - overriding the response writer, so that when w.Write([]byte) is called it prints to the log, what do you think of that? At least you don't need to stuff the response into the context

[ardan-bkennedy]

Then you are moving that in foundation, which is fine if that is what you want.

On Oct 22, 2021, at 11:44 PM, Jason Lui @.***> wrote:

Sorry for keeping coming back, I've seen another way of doing this - overriding the response writer, so that when w.Write([]byte) is called it prints to the log, what do you think of that? At least you don't need to stuff the response into the context

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/ardanlabs/service/issues/198#issuecomment-950050930, or unsubscribe https://github.com/notifications/unsubscribe-auth/AARMURK2WBOGBG5KQBVDA4DUIIVTHANCNFSM5GRBFVKQ. Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

[mrkagelui]

Then you are moving that in foundation, which is fine if that is what you want. … On Oct 22, 2021, at 11:44 PM, Jason Lui @.***> wrote: Sorry for keeping coming back, I've seen another way of doing this - overriding the response writer, so that when w.Write([]byte) is called it prints to the log, what do you think of that? At least you don't need to stuff the response into the context — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#198 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AARMURK2WBOGBG5KQBVDA4DUIIVTHANCNFSM5GRBFVKQ. Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

not necessarily in fact. I can have a "enhancedWriter" in logger middleware, and inject it like

func (r *loggingWriter) Write(data []byte) (int, error) {
    r.SaveIntoDB("response")
    ...
}

func Logger() web.Middleware {
    return func(handler web.Handler) web.Handler {
        return func(ctx context.Context, w http.ResponseWriter, r *http.Request) error {
            // Log incoming request
            logger.Info("request...")

            lw := loggingWriter{...}
            return handler(ctx, &lw, r)
        }
    }
}

[ardan-bkennedy]

Get it working and then send me an email on the side. I will review it

On Oct 23, 2021, at 10:47 AM, Jason Lui @.***> wrote:

Then you are moving that in foundation, which is fine if that is what you want. … <x-msg://141/#> On Oct 22, 2021, at 11:44 PM, Jason Lui @.***> wrote: Sorry for keeping coming back, I've seen another way of doing this - overriding the response writer, so that when w.Write([]byte) is called it prints to the log, what do you think of that? At least you don't need to stuff the response into the context — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#198 (comment) https://github.com/ardanlabs/service/issues/198#issuecomment-950050930>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AARMURK2WBOGBG5KQBVDA4DUIIVTHANCNFSM5GRBFVKQ https://github.com/notifications/unsubscribe-auth/AARMURK2WBOGBG5KQBVDA4DUIIVTHANCNFSM5GRBFVKQ. Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

not necessarily in fact. I can have a "enhancedWriter" in logger middleware, and inject it like

func (r *loggingWriter) Write(data []byte) (int, error) { r.SaveIntoDB("response") ... }

func Logger() web.Middleware { return func(handler web.Handler) web.Handler { return func(ctx context.Context, w http.ResponseWriter, r *http.Request) error { // Log incoming request logger.Info("request...")

      lw := loggingWriter{...}
      return handler(ctx, &lw, r)
  }

} } — You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/ardanlabs/service/issues/198#issuecomment-950163305, or unsubscribe https://github.com/notifications/unsubscribe-auth/AARMURPPMKXOTHKEDOJTOADUILDJ5ANCNFSM5GRBFVKQ. Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

[mrkagelui]

haha, sure, another POC PR then